907243 - There's no way to call SetDPI on AsyncPanZoomController through APZCTreeManager

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[Jim Mathies [:jimm]]

AFAICT, we are working with an APZCTreeManager down in metro widget, so there's no way to set the dpi properly. We probably don't want to go fishing to apz objects to do this, I think what we need here is the ability to set the dpi on the main APZCTreeManager, and AsyncPanZoomController's would then get their dpi settings from that. APZCTreeManager would also have to flush dpi settings out.

Comment 1

[Jim Mathies [:jimm]]

Attachment: move dpi to tree mgr patch v.1

Comment 2

[Jim Mathies [:jimm]]

I'll just land metro pref here and resolve the parent bug with this one.

Comment 3

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 792990 [details] [diff] [review]
move dpi to tree mgr patch v.1

Review of attachment 792990 [details] [diff] [review]:
-----------------------------------------------------------------

I'm not 100% sure this is right, because we might want different DPIs on different parts of the layer tree. Is the DPI here always a hardware DPI? Or is it related to the AppUnitsPerDevPixel scale factor?

Comment 4

[Jim Mathies [:jimm]]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #3)
> Comment on attachment 792990 [details] [diff] [review]
> move dpi to tree mgr patch v.1
> 
> Review of attachment 792990 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I'm not 100% sure this is right, because we might want different DPIs on
> different parts of the layer tree. Is the DPI here always a hardware DPI? Or
> is it related to the AppUnitsPerDevPixel scale factor?

The way I interpreted this was hardware dpi since we currently use this value for a couple of input hysteresis tests. The input metro sends to the apz is in device pixels and there doesn't appear to be any physical to logical conversion taking place on the receiving side.

As far as different dpis per apz, I'd be surprised if we wanted that. But I'm pretty new to this code, so I might be wrong.

Comment 5

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 792990 [details] [diff] [review]
move dpi to tree mgr patch v.1

Review of attachment 792990 [details] [diff] [review]:
-----------------------------------------------------------------

Ok, looks fine then. Doesn't look like it should break any existing behaviour at any rate.

::: gfx/layers/composite/APZCTreeManager.h
@@ +280,5 @@
>     * lock also needs to be held when accessing the mRootApzc instance variable, as that
>     * is considered part of the APZC tree management state. */
>    mozilla::Monitor mTreeLock;
>    nsRefPtr<AsyncPanZoomController> mRootApzc;
> +  static float sDPI;

nit: Please add a blank line above sDPI

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/6bedadef5f13