Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid rebuilding models for ListQuantityGroup and ListTextGroup #1338

Open
blammit opened this issue Jul 16, 2024 · 0 comments · May be fixed by #1903
Open

Avoid rebuilding models for ListQuantityGroup and ListTextGroup #1338

blammit opened this issue Jul 16, 2024 · 0 comments · May be fixed by #1903

Comments

@blammit
Copy link
Contributor

blammit commented Jul 16, 2024

ListQuantityGroup and ListTextGroup models are assembled using models like this:

ListQuantityGroup {
    text: "Multiple quantities"
    textModel: [
        { value: 5.1, unit: VenusOS.Units_Volt_DC },
        { value: 48, unit: VenusOS.Units_Watt }
    ]
}

This is conveniently straightforward to specify, but is problematic because when the quantity values change, the array is rebuilt, which in turn causes the delegates to be rebuilt.

So instead, the model should be a model or list object that does not rebuild its list entries when a value is modified.

Note: ListRadioButtonGroup has a similar issue with its optionModel, but the list values in those models rarely change (if at all) so it's probably fine to keep those as-is.

Solution 1

A simple solution is to use a list property of QObjects instead:

ListQuantityGroup {
    text: "Multiple quantities"
    textModel: [
        QuantityObject { value: 5.1; unit: VenusOS.Units_Volt_DC }
        QuantityObject { value: 48; unit: VenusOS.Units_Watt }
    ]
}


// ListQuantityGroup.qml
ListItem {
    property list<QuantityInfo> textModel
}

But there is still a problem where non-visible items need to be hidden in the group. (See #1087 (comment)) For example, currently we do this:

ListQuantityGroup {
    text: "Multiple quantities"
    textModel: [
        { value: someVeItem.value, unit: VenusOS.Units_Watt; visible: someVeItem.isValid },  // hide this conditionally
        { value: 5.1, unit: VenusOS.Units_Volt_DC },
    ]
}

This is problematic as the separator between the two items is not hidden as expected.

Solution 2

Perhaps a more general-purpose solution would be better, where we use ObjectModel with QuantityLabel:

ListQuantityGroup {
    model: ObjectModel {
        QuantityLabel { value: someVeItem.value, unit: VenusOS.Units_Watt; visible: someVeItem.isValid }
        QuantityLabel { value: 5.1, unit: VenusOS.Units_Volt_DC },
    }
}

This would still cause problems with separators if some of the labels should be non-visible. But if a VisibleItemModel is added then we can use that instead of ObjectModel, and omit the visible condition:

ListQuantityGroup {
    model: VisibleItemModel {
        QuantityLabel { value: someVeItem.value, unit: VenusOS.Units_Watt }
        QuantityLabel { value: 5.1, unit: VenusOS.Units_Volt_DC },
    }
}

This would help resolve the issue where separators are shown unnecessarily.

The downside is that QuantityLabel will be created even if an item will not be shown if it is not visible.

Solution 3

Provide a model that automatically hides undefined quantity values.

ListQuantityGroup {
    model: QuantityInfoModel {
        QuantityInfo { value: someVeItem.value, unit: VenusOS.Units_Watt }
        QuantityInfo { value: 5.1, unit: VenusOS.Units_Volt_DC },
    }
}

This way, there is no need to create QuantityLabel instances for items that will not be shown.

However, we also need a solution for non-number items so that ListTextGroup can do something similar.

@blammit blammit added this to the Version 1.1.0 milestone Jul 16, 2024
@blammit blammit changed the title Avoid array-based models for ListQuantityGroup and ListTextGroup Avoid rebuilding models for ListRadioButtonGroup , ListQuantityGroup and ListTextGroup Oct 2, 2024
@blammit blammit changed the title Avoid rebuilding models for ListRadioButtonGroup , ListQuantityGroup and ListTextGroup Avoid rebuilding models for ListQuantityGroup and ListTextGroup Oct 2, 2024
blammit added a commit that referenced this issue Nov 25, 2024
Instead of using a single _deviceDisplayInfo() function to configure
its delegates, load per-service files that define their own delegate
UIs.

The issue with the _deviceDisplayInfo() is that it causes binding
updates to all delegates whenever any of the displayed delegate values
change. The per-service delegate solution also allows custom list
items for individual services, instead of requiring all services to
use a similar delegate UI.

All delegates should have the same appearance as before, except for
the Inverter delegate, which has been fixed to only show the power for
the currently active phase.

Issue #1338
blammit added a commit that referenced this issue Nov 25, 2024
The _deviceDisplayInfo() in DeviceListPage is problematic as it causes
binding updates to all delegates whenever any of the displayed
delegate values change.

Instead of using this single gigantic function to configure delegates,
load per-service QML files that define their own delegate UIs. This
improvement also means that custom list items can be provided for
individual services, instead of requiring all services to use a
similar delegate UI.

All delegates should have the same appearance as before, except for
the Inverter delegate, which has been fixed to only show the power for
the currently active phase.

Issue #1338
blammit added a commit that referenced this issue Nov 25, 2024
The _deviceDisplayInfo() in DeviceListPage is problematic as it causes
binding updates to all delegates whenever any of the displayed
delegate values change.

Instead of using this single gigantic function to configure delegates,
load per-service QML files that define their own delegate UIs. This
improvement also means that custom list items can be provided for
individual services, instead of requiring all services to use a
similar delegate UI.

All delegates should have the same appearance as before, except for
the Inverter delegate, which has been fixed to only show the power for
the currently active phase.

Issue #1338
blammit added a commit that referenced this issue Nov 26, 2024
The _deviceDisplayInfo() in DeviceListPage is problematic as it causes
binding updates to all delegates whenever any of the displayed
delegate values change.

Instead of using this single gigantic function to configure delegates,
load per-service QML files that define their own delegate UIs. This
improvement also means that custom list items can be provided for
individual services, instead of requiring all services to use a
similar delegate UI.

All delegates should have the same appearance as before, except for
the Inverter delegate, which has been fixed to only show the power for
the currently active phase.

Issue #1338
blammit added a commit that referenced this issue Nov 26, 2024
The _deviceDisplayInfo() in DeviceListPage is problematic as it causes
binding updates to all delegates whenever any of the displayed
delegate values change.

Instead of using this single gigantic function to configure delegates,
load per-service QML files that define their own delegate UIs. This
improvement also means that custom list items can be provided for
individual services, instead of requiring all services to use a
similar delegate UI.

All delegates should have the same appearance as before, except for
the Inverter delegate, which has been fixed to only show the power for
the currently active phase.

Issue #1338
@blammit blammit self-assigned this Jan 27, 2025
blammit added a commit that referenced this issue Jan 31, 2025
QuantityObject provides QObject-based access to a quantity value.
QuantityObjectModel provides a model of QuantityObject values; it
filters out objects without valid values.

This allows ListQuantityGroup and its derived types to specify their
models without creating a JavaScript array that directly refers each
quantity value. Otherwise, whenever a quantity value is updated, the
entire array is rebuilt, which in turn causes the QuantityRow repeater
to rebuild its delegates.

Part of #1338
blammit added a commit that referenced this issue Jan 31, 2025
…Group

This improves the performance of QuantityRow in updating its displayed
values, as the Repeater no longer rebuilds its delegates whenever a
single quantity value changes.

The updates affect the following types and their uses:
- ListQuantityGroup
- ...

Replace all uses of ListTextGroup with ListQuantityGroup, except where
the text is static, or in PageTankShape where the list is dynamically
generated from an array.

[WIP]
Still to do:
- verify all changes
- update uses of quantityModel from device list delegates
- add filter option to QuantityObjectModel to disable filtering in table mode
- rebase on top of SolarInputListPage PR
- update uses of QuantityRow outside of ListQuantityGroup (e.g. battery list page)
- update PagePowerDebug

Part of #1338
blammit added a commit that referenced this issue Feb 11, 2025
Use QuantityTable as ListTextGroup will be obsoleted.

Use BackendConnection.uidPrefix() to load the page in mock mode.

Part of #1338
blammit added a commit that referenced this issue Feb 11, 2025
Use QuantityTable as ListTextGroup will be obsoleted.

Use BackendConnection.uidPrefix() to load the page in mock mode.

Part of #1338
blammit added a commit that referenced this issue Feb 11, 2025
Use QuantityTable as ListTextGroup will be obsoleted.

Use BackendConnection.uidPrefix() to load the page in mock mode.

Part of #1338
blammit added a commit that referenced this issue Feb 12, 2025
QuantityObject provides QObject-based access to a quantity value.
QuantityObjectModel provides a model of QuantityObject values, and
is able to filter out items that do not have valid values.

This allows ListQuantityGroup and its derived types to specify their
models without creating a JavaScript array that directly refers each
quantity value. Otherwise, whenever a quantity value is updated, the
entire array is rebuilt, which in turn causes the QuantityRow repeater
to rebuild its delegates.

Part of #1338
blammit added a commit that referenced this issue Feb 12, 2025
Change QuantityRow's model type to QuantityObjectModel, so that the
model cannot be specified as an array of real values, and update the
codebase accordingly so that it always uses QuantityObject with
QuantityObjectModel instead of an array-based model in all cases where
multiple quantities are displayed.

This greatly improves the performance of QuantityRow in updating its
values, as the Repeater no longer rebuilds its delegates whenever a
single quantity changes in the model.

Also, since QuantityObjectModel is able to filter out invalid values,
it is not necessary to create and show separate models depending on
whether particular values are valid. (In the DC input and solar input
Overview drilldowns, where invalid values should still be shown due to
the table-like display, set QuantityObjectModel.NoFilter instead of
tableMode=true to preserve the table format.)

The updates affect the following types and their uses:
- QuantityRow
- ListQuantityGroup
- ListQuantityGroupNavigation

Also remove ListTextGroup and replace its uses with ListQuantityGroup,
as ListTextGroup was being used to show text alongside number values,
and that can now easily be done with ListQuantityGroup as a
QuantityObject value can be a string if the unit is Units_None.

Part of #1338
blammit added a commit that referenced this issue Feb 12, 2025
Change QuantityRow's model type to QuantityObjectModel, so that the
model cannot be specified as an array of real values, and update the
codebase accordingly so that it always uses QuantityObject with
QuantityObjectModel instead of an array-based model in all cases where
multiple quantities are displayed.

This greatly improves the performance of QuantityRow in updating its
values, as the Repeater no longer rebuilds its delegates whenever a
single quantity changes in the model.

Also, since QuantityObjectModel is able to filter out invalid values,
it is not necessary to create and show separate models depending on
whether particular values are valid. (In the DC input and solar input
Overview drilldowns, where invalid values should still be shown due to
the table-like display, set QuantityObjectModel.NoFilter instead of
tableMode=true to preserve the table format.)

The updates affect the following types and their uses:
- QuantityRow
- ListQuantityGroup
- ListQuantityGroupNavigation

Also remove ListTextGroup and replace its uses with ListQuantityGroup,
as ListTextGroup was being used to show text alongside number values,
and that can now easily be done with ListQuantityGroup as a
QuantityObject value can be a string if the unit is Units_None.

Part of #1338
blammit added a commit that referenced this issue Feb 12, 2025
Change QuantityRow's model type to QuantityObjectModel, so that the
model cannot be specified as an array of real values, and update the
codebase accordingly so that it always uses QuantityObject with
QuantityObjectModel instead of an array-based model in all cases where
multiple quantities are displayed.

This greatly improves the performance of QuantityRow in updating its
values, as the Repeater no longer rebuilds its delegates whenever a
single quantity changes in the model.

Also, since QuantityObjectModel is able to filter out invalid values,
it is not necessary to create and show separate models depending on
whether particular values are valid. (In the DC input and solar input
Overview drilldowns, where invalid values should still be shown due to
the table-like display, set QuantityObjectModel.NoFilter instead of
tableMode=true to preserve the table format.)

The updates affect the following types and their uses:
- QuantityRow
- ListQuantityGroup
- ListQuantityGroupNavigation

Also remove ListTextGroup and replace its uses with ListQuantityGroup,
as ListTextGroup was being used to show text alongside number values,
and that can now easily be done with ListQuantityGroup as a
QuantityObject value can be a string if the unit is Units_None.

Part of #1338
blammit added a commit that referenced this issue Feb 13, 2025
QuantityObject provides QObject-based access to a quantity value.
QuantityObjectModel provides a model of QuantityObject values, and
is able to filter out items that do not have valid values.

This allows ListQuantityGroup and its derived types to specify their
models without creating a JavaScript array that directly refers each
quantity value. Otherwise, whenever a quantity value is updated, the
entire array is rebuilt, which in turn causes the QuantityRow repeater
to rebuild its delegates.

Part of #1338
blammit added a commit that referenced this issue Feb 13, 2025
Change QuantityRow's model type to QuantityObjectModel, so that the
model cannot be specified as an array of real values, and update the
codebase accordingly so that it always uses QuantityObject with
QuantityObjectModel instead of an array-based model in all cases where
multiple quantities are displayed.

This greatly improves the performance of QuantityRow in updating its
values, as the Repeater no longer rebuilds its delegates whenever a
single quantity changes in the model.

Also, since QuantityObjectModel is able to filter out invalid values,
it is not necessary to create and show separate models depending on
whether particular values are valid.

The updates affect the following types and their uses:
- QuantityRow
- ListQuantityGroup
- ListQuantityGroupNavigation

Also remove ListTextGroup and replace its uses with ListQuantityGroup,
as ListTextGroup was being used to show text alongside number values,
and that can now easily be done with ListQuantityGroup as a
QuantityObject value can be a string if the unit is Units_None.

Part of #1338
blammit added a commit that referenced this issue Feb 13, 2025
Change QuantityRow's model type to QuantityObjectModel, so that the
model cannot be specified as an array of real values, and update the
codebase accordingly so that it always uses QuantityObject with
QuantityObjectModel instead of an array-based model in all cases where
multiple quantities are displayed.

This greatly improves the performance of QuantityRow in updating its
values, as the Repeater no longer rebuilds its delegates whenever a
single quantity changes in the model.

Also, since QuantityObjectModel is able to filter out invalid values,
it is not necessary to create and show separate models depending on
whether particular values are valid.

The updates affect the following types and their uses:
- QuantityRow
- ListQuantityGroup
- ListQuantityGroupNavigation

Also remove ListTextGroup and replace its uses with ListQuantityGroup,
as ListTextGroup was being used to show text alongside number values,
and that can now easily be done with ListQuantityGroup as a
QuantityObject value can be a string if the unit is Units_None.

Part of #1338
blammit added a commit that referenced this issue Feb 13, 2025
QuantityObject provides QObject-based access to a quantity value.
QuantityObjectModel provides a model of QuantityObject values, and
is able to filter out items that do not have valid values.

This allows ListQuantityGroup and its derived types to specify their
models without creating a JavaScript array that directly refers each
quantity value. Otherwise, whenever a quantity value is updated, the
entire array is rebuilt, which in turn causes the QuantityRow repeater
to rebuild its delegates.

Part of #1338
blammit added a commit that referenced this issue Feb 13, 2025
Change QuantityRow's model type to QuantityObjectModel, so that the
model cannot be specified as an array of real values, and update the
codebase accordingly so that it always uses QuantityObject with
QuantityObjectModel instead of an array-based model in all cases where
multiple quantities are displayed.

This greatly improves the performance of QuantityRow in updating its
values, as the Repeater no longer rebuilds its delegates whenever a
single quantity changes in the model.

Also, since QuantityObjectModel is able to filter out invalid values,
it is not necessary to create and show separate models depending on
whether particular values are valid.

The updates affect the following types and their uses:
- QuantityRow
- ListQuantityGroup
- ListQuantityGroupNavigation

Also remove ListTextGroup and replace its uses with ListQuantityGroup,
as ListTextGroup was being used to show text alongside number values,
and that can now easily be done with ListQuantityGroup as a
QuantityObject value can be a string if the unit is Units_None.

Part of #1338
blammit added a commit that referenced this issue Feb 13, 2025
Change QuantityRow's model type to QuantityObjectModel, so that the
model cannot be specified as an array of real values, and update the
codebase accordingly so that it always uses QuantityObject with
QuantityObjectModel instead of an array-based model in all cases where
multiple quantities are displayed.

This greatly improves the performance of QuantityRow in updating its
values, as the Repeater no longer rebuilds its delegates whenever a
single quantity changes in the model.

Also, since QuantityObjectModel is able to filter out invalid values,
it is not necessary to create and show separate models depending on
whether particular values are valid.

The updates affect the following types and their uses:
- QuantityRow
- ListQuantityGroup
- ListQuantityGroupNavigation

Also remove ListTextGroup and replace its uses with ListQuantityGroup,
as ListTextGroup was being used to show text alongside number values,
and that can now easily be done with ListQuantityGroup as a
QuantityObject value can be a string if the unit is Units_None.

Part of #1338
blammit added a commit that referenced this issue Feb 13, 2025
Change QuantityRow's model type to QuantityObjectModel, so that the
model cannot be specified as an array of real values, and update the
codebase accordingly so that it always uses QuantityObject with
QuantityObjectModel instead of an array-based model in all cases where
multiple quantities are displayed.

This greatly improves the performance of QuantityRow in updating its
values, as the Repeater no longer rebuilds its delegates whenever a
single quantity changes in the model.

Also, since QuantityObjectModel is able to filter out invalid values,
it is not necessary to create and show separate models depending on
whether particular values are valid.

The updates affect the following types and their uses:
- QuantityRow
- ListQuantityGroup
- ListQuantityGroupNavigation

Also remove ListTextGroup and replace its uses with ListQuantityGroup,
as ListTextGroup was being used to show text alongside number values,
and that can now easily be done with ListQuantityGroup as a
QuantityObject value can be a string if the unit is Units_None.

Part of #1338
blammit added a commit that referenced this issue Feb 13, 2025
QuantityObject provides QObject-based access to a quantity value.
QuantityObjectModel provides a model of QuantityObject values, and
is able to filter out items that do not have valid values.

This allows ListQuantityGroup and its derived types to specify their
models without creating a JavaScript array that directly refers each
quantity value. Otherwise, whenever a quantity value is updated, the
entire array is rebuilt, which in turn causes the QuantityRow repeater
to rebuild its delegates.

Part of #1338
blammit added a commit that referenced this issue Feb 13, 2025
Change QuantityRow's model type to QuantityObjectModel, so that the
model cannot be specified as an array of real values, and update the
codebase accordingly so that it always uses QuantityObject with
QuantityObjectModel instead of an array-based model in all cases where
multiple quantities are displayed.

This greatly improves the performance of QuantityRow in updating its
values, as the Repeater no longer rebuilds its delegates whenever a
single quantity changes in the model.

Also, since QuantityObjectModel is able to filter out invalid values,
it is not necessary to create and show separate models depending on
whether particular values are valid.

The updates affect the following types and their uses:
- QuantityRow
- ListQuantityGroup
- ListQuantityGroupNavigation

Also remove ListTextGroup and replace its uses with ListQuantityGroup,
as ListTextGroup was being used to show text alongside number values,
and that can now easily be done with ListQuantityGroup as a
QuantityObject value can be a string if the unit is Units_None.

Part of #1338
blammit added a commit that referenced this issue Feb 14, 2025
Change QuantityRow's model type to QuantityObjectModel, so that the
model cannot be specified as an array of real values, and update the
codebase accordingly so that it always uses QuantityObject with
QuantityObjectModel instead of an array-based model in all cases where
multiple quantities are displayed.

This greatly improves the performance of QuantityRow in updating its
values, as the Repeater no longer rebuilds its delegates whenever a
single quantity changes in the model.

Also, since QuantityObjectModel is able to filter out invalid values,
it is not necessary to create and show separate models depending on
whether particular values are valid.

The updates affect the following types and their uses:
- QuantityRow
- ListQuantityGroup
- ListQuantityGroupNavigation

Also remove ListTextGroup and replace its uses with ListQuantityGroup,
as ListTextGroup was being used to show text alongside number values,
and that can now easily be done with ListQuantityGroup as a
QuantityObject value can be a string if the unit is Units_None.

Part of #1338
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant