suspicious of dp/px

[richard-uk1]

I think that sometimes we are returning measurements in device points when we should be using physical pixels, and vice versa. winit use some newtype wrappers to make the two types of measurement distinct in the type system. Whether we copy this or not, we should audit the places that take or return measurements to make sure they are using the right units.

As a specific example, I think WindowHandle::get_size is returning physical pixels when the docs say device points.

[raphlinus]

I poked at this just a little, focusing on the mac backend (it may well be that Windows works better). I believe the culprit is the mac backend implementation of get_scale, which has a TODO and just returns 1.0 for dpi scaling. This is not a glazier-specific problem, the code is the same as druid-shell. I believe the most likely explanation is that druid-shell also gets the dpi scale factor wrong, but then the Core Graphics back-end for piet applies the correct scale factor for rasterization, so it doesn't matter much.

The fix for this is for get_scale to return the dpi scaling factor and for get_size to return physical pixels (ie display points * scale).

[ghost]

Is there agreement on generally moving to newtypes for these units though? To encode this information in the type system and avoid these kind of errors?

[raphlinus]

I don't think we should newtype, it wouldn't have solved this problem. Display points (aka px) and device pixels are not different units, they're a different coordinate frame. And we have lots of coordinate frames (window relative, screen relative, parent relative) etc, and this doesn't encode easily in the Rust type system.

[ghost]

So these aren't equivalent to winit's LogicalSize and PhysicalSize?

[raphlinus]

If we did newtypes for size in display points and hardware pixels, they would be basically equivalent to the winit types, but I don't think it's worth it.

[raphlinus]

I was also experimenting with Windows, and dpi scaling has issues there as well. As far as I can tell, the value reported to the WinHandler::size() callback is correctly in display points, but WindowHandle::get_size() is actually returning hardware pixels (see comment on backend impl). This was introduced in linebender/druid#1713, where some of the methods were updated but get_size was skipped. I think Druid doesn't actually use get_size, but the xilem prototype does. (This is also perhaps something we should reconsider, arguably the configuration of render resources should be edge-triggered by a size() callback rather than level-triggered, but either way the call should do what it says)

One last thing. I think it's highly confusing that we use "px" for hardware pixels, while in CSS the "px" identifier means the same concept we're calling "display points." I'm ok with "dp" because at least it's consistent with Android, but I suggest we use "pxl" or "pixel" instead.

[raphlinus]

Now that #36 has landed, I'm wondering whether this can be closed. I'm not sure things are perfect (particularly in the dynamic case), but I think we have parity with the Druid state.

[richard-uk1]

Yes let's close - I'll open something more specific if I continue to get issues.