Change the way CN1 handle device resolution to use device PPI

[ramsestom]

This pull request makes changes into how CN1 handle device resolution.
It makes it use device PPI (pixels per inch) wich is kind of a standard to express screen resolution (instead of the pixels/mm values previously used by CN1 which I never saw anywhere else). It has the advantage of beeing a system of measurment familiar to developpers and to allow most precise mesures/conversions than a pixels/mm int value.
Note that I kept the source of the old getDeviceDensity() function into the CodenameOneImplementations, but I tagged it as deprecated and reduced its visibility to protected. I did that because I know you are kind of reluctant to suppress functions from the CN1 code. But, with this pull request, this getDeviceDensity() function is no longer used anywhere now so I would be in favour to completely remove it.
I also kept the DENSITY_xxx constants (into CN1Constants) for now. They are no longer used by th CN1 source code (exept into the getDeviceDensity() methods but that could be deleted) but are used by the CodenameOneDesigner (for MultipleImages). If this pull request is accepted (which I hope ;) ), it would be good to also replace usage of DENSITY_xxx into the CodenameOneDesigner with the new DPI_xxx constants (that would make more sense anyway to have each image into a MultipleImage associated to its PPI resolution rather than to its pixels/mm resolution). Note that MultipleImage indexed with the old system of pixels/mm resolution would still work with the new code though (pixels/mm are converted to PPI when we read the ressource)
Also I used the DPI acronym to express PPI, to keep consistency with the rest of the CN1 code (where you use DPI to express PPI, even if this is an abuse of language (https://en.99designs.fr/blog/tips/ppi-vs-dpi-whats-the-difference/)). But you can refactor the names I used for variables and functions to your convenience anyway

This pull request also introduce the dp and sp mesure units (https://material.io/design/layout/understanding-layout.html#pixel-density) and makes the necessary changes into Display, Style and LayeredLayout so that they can be used.

Important remark: I tested my code into the simulator and on Android but I couldn't test it on iOS as I didn't have a mac to compille at hand... So there might be some issues in the native iOS part of my modifications (especially since I really do not master objectiveC). Sorry for that

[codenameone]

Thanks for the PR. It's very well written & I like a lot of the ideas within it!

It does touch a few core/sensitive pieces of functionality that can break applications all over the place with minor behavior changes. So this is a very risky commit, even more so than the previous PR.

There are a lot of things I like in this commit had they been separate. While I'm not a fan of device detection code in general, the convertToPixels() change for the iOS port makes a lot of sense.

Adding the SP/DP units as an option wouldn't bother anyone so it's a decent option to add as it won't break code.

I have several concerns and questions about this:

• Why did you deprecate the existing way to obtain density?
• How does this handle devices that are unknown during compile time?
• Densities in iOS seem incorrect, there should be 3 densities and you seem to only have 2?
• How does the device density behavior map to the simulator correctly and the quick add multi-image logic?

The way I see this you are trying to address several separate issues with one PR, I suggest a more iterative approach that would help us manage potential regressions/issues.

I also prefer not to deprecate API's even if they aren't great. This lets us seamlessly roll out improvement without making our users rework their logic.

[ramsestom]

Yes sorry, I probably should have make 2 different PR (but as I wrote this all at once and the SP/DP part depends on the changes made on the density one it was just more convenient for me to make a unique commit to my repository). If you want to focus first into the density changes, you can omit the Style, StyleParser and LayeredLayout files (that contain changes for SP/DP)

So to make some clarifications on how this PR works:
Currently, the function to obtain device density (i.e. getDeviceDensity() ) is only used for two things in CN1: determine the convertion factor from mm to pixels and choose the correct image into a MultipleImage . That is the reason why, if you remove the getDeviceDensity() methods and DENSITY_xxx constants into this PR, you won't see any compilation issue for the CN1 core (only the CodenameOneDesigner will be affected, has I already pointed out as it still use some DENSITY_xxx constants) as I replaced these usage with the new getDeviceDPI() method this PR introduce.

Why did you deprecate the existing way to obtain density?

The reason I deprecate the existing way to obtain density is that CN1 code do no longer need it and I think it is misleading to the developpers. When calling a getDeviceDensity() method, any developper would assume to obtain a density value (or factor) into PPI. Here it is not the case. Furthermore, the getDeviceDensity() method returns rough pixels/mm density estimations that could be not perfectly consistent with the more precise PPI value returned by getDeviceDPI(). So keeping the two methods for returning the density would requiere to keep them in sync and might just be a burden for the future.
Yet, If you want to keep the old getDeviceDensity() method, just redeclare it public into CodenameOneImpl

How does this handle devices that are unknown during compile time?

It does it exactly the same way the old method was handling it. The only thing I changed is that instead of returning an estimate of the density (for the getDeviceDensity() method), the getDeviceDPI() method returns the exact device density or a really close one when this information is known. So for iOS, the exact PPI density would be returned, for android, a really close one based one one the densityDpi density bucket value (I abandonned the idea of using xdpi and ydpi as too much devices have errors on these and android now offers many DENSITY_xxx buckets so the value returned by densityDpi is quite close to the reality anyway) and for other devices, it would be a rough estimation based on screen resolution.

Densities in iOS seem incorrect, there should be 3 densities and you seem to only have 2?

You mean for the iPhone part? Because there is actually 3 different densities for the iPads (I added a new test to differentiate iPad retina from other iPads) and 3 for the iPhones.
In the getDeviceDensity() the iPhones have 4 different densities but I think it is incorrect, iphoneX and iphones 6+ -> 8+ should have the same (see: https://www.paintcodeapp.com/news/ultimate-guide-to-iphone-resolutions and https://en.wikipedia.org/wiki/List_of_iOS_devices#iPhone). Note that I kept the "if(largest > 1300)" test to differentiate iphones 6 -> 8 from iphones 4 and 5 even if it is useless as they have the same density at the end (so you can simplify this part of the code if you want)

How does the device density behavior map to the simulator correctly and the quick add multi-image logic?

For the simulator, it is exactly the same behaviour as previously. Nothing changed
For multi-images, as those created whith the CodenameOneDesigner still use old DENSITY_xxx values to tag the density of each image, I made kind of an hack that test, when the resource is read, if the density value of this image is <= 80 (in which case it is really unlikely to be a PPI value but rather a pixels/mm value) and if it is a known old DENSITY_xxx constant, in which case this pixels/mm value is converted into PPI (maybe I could have made a test a bit more "smart" and test if every image of a multiimage has a density <=80 in which case it would switch to a pixels/mm to PPI conversion mode rather than testing densities one by one but I think the current aproach should be OK). So now, you can use any PPI value to tag an image of a multipleimage (not just one supported by the PPI_xxx constants) and it should work.
Also, I changed the method to choose the best image given a device density a bit. Now I choose the image which has the density closer to the device density rather than the closest one with a density equals or lower (as you can have a device density just a few pixels higher than the one of your image, for example a 163 PPI on iPhone 3 with a 160 PPI resource image)

[codenameone]

With the iPhone there are 3 densities illustrated in the image you linked @1x, @2x & @3x. You seem to treat 3x and 2x as one density.
You'll notice that the current IOSImplementation supports 3 density types not two for phones.

I don't see the benefit of exposing a DPI value over the "virtual" values we currently have?
It's intended as a theoretical value not as an accurate representation. It's also something we allow overriding for cases such as games where DPI should act "differently".

I suggest breaking this PR to multiple smaller independent stages. Most of the things in it shouldn't be a problem. But messing with the DPI code for multi-images is really risky.

[ramsestom]

No I have 3 densities for iPhone and correctly treat @2x and @3x as different densities.
if you look at my code
@1x = dDPI = 163;
@2x = dDPI = 326;
@3x = dDPI = 458;

The benefit of exposing a DPI value rather than the current "virtual" value is that it allows the developpers to access a more precise and comprehensible value of the screen density. A developper may need this if he uses its own distance unit that he want to convert into pixels for example (imagine someone may want to use in, pt, em, css px, ex... units). Accessing a most precise as possible screen resolution value can be usefull to a developper in many ways. That is pretty much why Android increased its number of virtual DENSITY_xxx values over the years (at first there was only DENSITY_LOW, DENSITY_MEDIUM and DENSITY_HIGH).
And having the MultiImages densities expressed as DPI (i.e. PPI) instead of with virtual values is also an advantage is someone want to have fine grain images resources (curently you are limited to virtual densities supported by CN1) or if he want to port MultiImages ressources from another Framework (for example, if you have multi images resources from Android, each image would be assotiated with a DENSITY_xxx value that express its density into PPI and you would have to find the matching CN1 virtual density if you want to create a suitable CN1 MultiImage from these)
Anyway, at some point, the fact is you need to convert from a screen DPI density value to CN1 virtual density value. Currently CN1 does this convertion upstream, before returning the virtual value with getDeviceDensity(). (For example the known 163 PPI IPhone 3Gs screen is mapped to Display.DENSITY_MEDIUM), then this virtual density is used to choose the correct image from a multiImage ressource. There is no real reason to apply this "rounding" logic upstream rather than just before necessary. This is exactly what this PR is doing (it "round" the device DPI density to the closest available image density).
The advantage of that is that you keep the most precise screen density value (the DPI) as deep as you can without loosing information and don't endup having to make approximations every time you need information on the screen density (like for converting mm to pixels in convertToPixels() or in the getPlatformDPI() methods of PlatformDefaults). And it also results in a far more easier to maintain code (you don't have to compute multiple conversion values and keep them in sync (currently, if you expressed them in the same unit, the virtual density value returned by getDeviceDensity() differ from the one returned by getPlatformDPI() and the conversion factor from mm to pixels used into convertToPixels() for example) every time you want to support a new "virtual" density).

[codenameone]

When we started off we didn't have accurate DPI values for many devices including Android (which as I mentioned gave bad results). We also added DPI's as we moved forward just like Google. I'd rather have a consistent predictable value for DPI (which as I mentioned, we can manipulate) than have rounding at the last minute.

The use cases you describe apply to convertToPixels you can add the flip method of convertToDips and also add a PPI estimate method. Those have separate logic and answer the use cases you describe perfectly. But the Density value is specifically designed for broad sweep assumptions/images. It works well and supports some hidden use cases (e.g. manipulating the DPI value for gaming) so flipping this would break some working code without providing a benefit over just adding API's.

[ramsestom]

The Density value is already derived, in any case, from the PPI value (because no platform directly express density in pixels/mm). If I understand you correctly, what you are saying is that you want to keep the getDensity() method to choose the correct image from multiImages but agree that the getDeviceDPI() method should be used for distances (mm, dp...) to pixels conversions. But what is the point of doing this? Basically you would be converting the PPI value natively returned by the Platform (or estimated) to a rouded pixels/mm density value that you will then use to choose the correct image. It is simpler to directly choose the correct image from the PPI value (and you can still manipulate/overwritte this PPI value the same way you did with the density one if you need to). Also expressing images resolutions in PPI is much natural and clear for developpers (even if multiimages expressed with old density values would still be supported under the hood).
Maybe I don't get some really specific use cases. Can you give a concrete example of a use case where you would need to overwrite the current getDensity() method (for games...?) and that would not work with this PR?

[codenameone]

Android & iOS have their own logic that separates the concept of density from the concept of which image to pick. So this is exactly how both of these operating systems work. You have constants that match a heuristic for picking the right image. This logic is delicate, it doesn't mean we shouldn't touch it but it means that every change we make to it needs to proceed with HUGE caution and a good reason because it can disrupt a lot of working code.
convertToPixels is also sensitive but it doesn't "skip" as much. If we get density wrong we can get a MUCH larger image but if convertToPixels returns 3 pixels where it previously returned 4 or 2 that won't break most UI's.

Making stuff clearer to developers is important but that's more of a javadoc/UI aspect not so much a constants change. We do have a table showing density mapping. It might be worth exposing those details in UI within the designer. In CSS multi-image density is expressed through DPI so that's already there.

In games density doesn't matter, only resolution matters so resources can be loaded with a fixed density based on game logic. I think it's implemented in the Poker demo.

[ramsestom]

OK I just performed some changes that should satisfy you I think. This allow to keep full back compatibility with the old DENSITY_xxx system (so if someone previously override getDeviceDensity() in its code or use MultiImages tagged with DENSITY_xxx values, it will work and the UI should remain unchanged) while allowing CN1 to move forward (use the PPI values as recommended density values and add support for densities in PPI in MultiImages which I think is the good direction to take)

[codenameone]

Thanks, I've merged it. Hopefully this won't break too much in this state.
@shannah can you run some tests too to see if something breaks?

[codenameone]

This was reverted due to #2429 we got multiple reports about a performance regression due to this commit. I have no idea what triggered that since it's really not obvious from the code.

[ramsestom]

OK. Since this is happening on Android, my first guess would be that this is due to the getDeviceDPI() method into AndroidImplementation. Actually, at each method call (which can happen quite frequently as this method is used to compute distances convertions like mm to px), a new call to android metrics would be made. I don't know if this is costly (probably as this seems to be the cause of the performances issue here). So the solution would be to cache the dpi value instead of requesting it from metrics each time. So changing the code to:

int dDPI = -1;
    
    @Override
    public int getDeviceDPI() {
        if (dDPI == -1) {
            DisplayMetrics metrics = new DisplayMetrics();
            if (getActivity() != null) {
                getActivity().getWindowManager().getDefaultDisplay().getMetrics(metrics);
            } else {
                metrics = getContext().getResources().getDisplayMetrics();
            }

            dDPI = metrics.densityDpi; // (int) (metrics.density * 160f);
        }
        return dDPI;
    }

might solve the problem.
I should have implemented this DPI value cache like for the iOS implementation. My bad

EDIT: I can't be 100% sure that the dpi cache implementation would solve the issue. So you should make some tests on your side before pushing it of course.
I tested on my android test device and did not see any performance issue (but I already tested with my first PR and did not noticed any issue either. The thing is that the forms I have implemented for now are quite simple and performance issues might only be noticed with complex ones (that have to perform many mm to px conversions)...)
And currently, getting device density on android with the getDeviceDensity() method use the same code that I think might be problematic BUT convertToPixels() only use this code:
DisplayMetrics dm = getContext().getResources().getDisplayMetrics(); and do not make a getActivity() call (which might be the cause of the issue). The thing is that only convertToPixels() is frequently called (while getDeviceDensity() is pretty much only used once, when loading ressources) which I think is why this performance issue do not appear with the current CN1 implementation (and should be fixed in my PR with the DPI cache implementation on android)