AsteroidOS / qml-asteroid

QML components, styles and demos for AsteroidOS.
GNU Lesser General Public License v2.1
53 stars 13 forks source link

`Dims` statement considered harmful / asteroid apps break when windows resized #44

Open dodoradio opened 1 year ago

dodoradio commented 1 year ago

Preamble

QML element sizing is generally expected to either be explicitly set, or hierarchical. This means that QML item size is generally set in one of four ways:

Dims doesn't align with any of the above.

The issue

Many asteroid apps use Dims as a means to ignore the QML hierarchy and directly get fractions of the size of the current window or the display. The window and display size are currently interchangeable because asteroid apps are always fullscreen, which allows this behaviour. Dims breaks in this usecase as soon as the window is resized, as on desktop platforms.

Solutions

The fantasy solution would be to have Dims give fractions of the current window, rather than the entire display size. This would easily fix all the current code.

Unfortunately, QML doesn't want to work this way. The Dims singleton would have to somehow be mapped to the root window of the application, and any implementation of this would also affect how Dims is used in code.

The distinction between h and w is also not clear. Generally apps are designed correctly, but it is possible to have applications behave in very strange ways in unconventional display geometries when h or w are incorrectly picked instead of l

The actual solution involves stopping the use of Dims to ignore the QML hierarchy. In most places, this will just mean replacing, for example, Dims.h(10) with windowID.height*0.1 (windowID being the id: which was assigned to the Application at the root of the app).

Flat tyres

Another current justification for Dims is that they provide a way to deal with flat tyres. I don't believe this should be used anywhere other than asteroid-launcher - and even there, it could be trivially replaced with Screen.desktopAvailableHeight+DeviceInfo.flatTireHeight.

I propose that a fractional value greater than one such as flatTireHeightFractional should be exposed through deviceInfo - this should make it far easier to deal with the flat tyre, without having to go through Dims or deal with the absolute display dimensions. This would be calculated as (Screen.desktopAvailableHeight+DeviceInfo.flatTireHeight)/Screen.desktopAvailableHeight, and would return a value such as 1.066667 on ayu. Dims.h(100) would be replaced with windowID.height*DeviceInfo.flatTireHeightFractional. This should eliminate the flat tyre usecase for Dims as well.

Perhaps some implementation details may differ for this, but the point is that Dims should not be used here either.

Does Dims still have uses?

The spirit of Dims does still have some value, as it provides an easy and consistent source of font and other element sizes. Asteroid is not currently DPI-aware, which makes the UI look very consistent across watches, but makes it weirdly massive on devices such as minnow and difficult to read on smaller devices such as sawfish. It may be desirable to add some interface for getting DPI-aware element sizes.

In its current form, Dims probably should not have a future. On desktop, it will always continue to break things; the lack of DPI awareness and assumption of fullscreen will always cause elements to be inappropriately oversized.

Dims will likely continue to be used internally in qml-asteroid to set the default element sizes. Provided that Dims is made DPI-aware, this should be fine on desktop as well.

beroset commented 1 year ago

I think this makes sense. As an experiment, I patched asteroid-calculator to eliminate Dims. It was actually quite easy and the results look just fine.
no-dims.diff.txt 20230508_150723

dodoradio commented 1 year ago

@beroset that's exactly the kind of changes I was looking at. Thank you! From the apps I have cloned on my machine, I can see that some other offenders include compass, flashlight, settings, weather and a massive chunk of the launcher. However, some of these, such as compass and flashlight, are using this to make the flat tyre look correct.

MagneFire commented 1 year ago

I too think that this make sense. Especially since we already are required to avoid Dims in watchfaces and applaunchers since a preview is shown in the settings application. Where the absolute dimensions can't be used.

It also seems that Dims was previously DPI aware as is evident with https://github.com/AsteroidOS/qml-asteroid/blob/21b8ab536c7216357baddb4dc29d848da921912c/src/controls/qml/Units.qml.

beroset commented 1 year ago

As a more complex exercise, I also converted asteroid-calendar to eliminate the use of Dims. asteroid-calendar.diff.txt

eLtMosen commented 1 year ago

The only hope i had with dims was that they would make it easier to adapt layouts to rectangular/irregular shaped screens using the Dims.l(XX). Since we have not yet adapted all of the OS for rectangular screens, could we define a method for that so i can implement it consistently?

beroset commented 1 year ago

Right now Dims.l() is defined like this:

    function l(number) {
        if(Screen.desktopAvailableWidth > (Screen.desktopAvailableHeight+DeviceInfo.flatTireHeight))
            return h(number)
        else
            return w(number)
    }

One obvious way to reimplement would be to simply require passing the height and width:

    function l(number, height, width) {
        return (width < height ? width : height) * number / 100
    }

But then it's such a trivial function, it's not clear that it adds much.

dodoradio commented 1 year ago

@eLtMosen Even simpler, I believe I've recommended this before: Math.max(height, width). You can easily make this accessible throughout the entire qml heirarchy by adding something like property double displayMaxDimension: Math.max(height,width) to the Application item at the root of the app.