Install on the device latest Gaia code with custom-prefs.js and this values: pref("layout.css.devPixelsPerPx", "1.6875"); When Gecko tries to translate some element vertically (via CSS) there is a flickering (the element goes up for a second) as it seems to does not take in count the new CSS pixels value

(In reply to Ismael from comment #0) > When Gecko tries to translate some element vertically (via CSS) there is a > flickering (the element goes up for a second) as it seems to does not take > in count the new CSS pixels value Need more information about the testcase. Does this involve transitions/animations?

Yes this happens when applying transitions/animations

Nick, can you reproduce this on an OMTA desktop build? If so, please fix it :-).

Basically, try OMTA testcases with full-zoom enabled so the ratio of CSS pixels to device pixels is not 1-1.

Yes, I can reproduce this. Now for the hard part...

I can repro this without throttling, but not without OMTA.

Comment on attachment 712097 [details] [diff] [review] fix Review of attachment 712097 [details] [diff] [review]: ----------------------------------------------------------------- Glad you tracked this down! ::: gfx/layers/ipc/PLayers.ipdl @@ +144,3 @@ > gfxPoint3D mozOrigin; > gfxPoint3D perspectiveOrigin; > nsRect bounds; Can you please add some comments documenting all these? ::: layout/base/nsDisplayList.cpp @@ +393,5 @@ > } > } > nsPoint origin = aItem->ToReferenceFrame(); > > + int32_t auPerCSSPixel = frame->PresContext()->AppUnitsPerDevPixel(); "auPerCSSPixel" doesn't seem like the right name here!

with comments and a name change

Comment on attachment 712283 [details] [diff] [review] fix Review of attachment 712283 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/layers/ipc/CompositorParent.cpp @@ +723,5 @@ > data.perspectiveOrigin(), > data.perspective()); > gfx3DMatrix transform = > nsDisplayTransform::GetResultingTransformMatrix(props, data.origin(), > nsDeviceContext::AppUnitsPerCSSPixel(), shouldn't this use appUnitsPerDevPixel as well? I'm pretty sure it should! ::: gfx/layers/ipc/PLayers.ipdl @@ +143,3 @@ > nsPoint origin; > + // origin in device pixels > + gfxPoint3D scaledOrigin; Why is this a gfxPoint3D? You only need x/y right? I think we really just want to pass appUnitsPerDevPixel here anyway.

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #10) > Comment on attachment 712283 [details] [diff] [review] > fix > > Review of attachment 712283 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: gfx/layers/ipc/CompositorParent.cpp > @@ +723,5 @@ > > data.perspectiveOrigin(), > > data.perspective()); > > gfx3DMatrix transform = > > nsDisplayTransform::GetResultingTransformMatrix(props, data.origin(), > > nsDeviceContext::AppUnitsPerCSSPixel(), > > shouldn't this use appUnitsPerDevPixel as well? I'm pretty sure it should! > I thought so too, but that animates incorrectly - when we use perDevPixel here it overcompensates the scale. > ::: gfx/layers/ipc/PLayers.ipdl > @@ +143,3 @@ > > nsPoint origin; > > + // origin in device pixels > > + gfxPoint3D scaledOrigin; > > Why is this a gfxPoint3D? You only need x/y right? > Because it gets used as a gfx3DPoint in CompositorParent, and I thought it was easier to do it this way than to convert it later... > I think we really just want to pass appUnitsPerDevPixel here anyway. ...but this is better in any case.

(In reply to Nick Cameron [:nrc] from comment #11) > I thought so too, but that animates incorrectly - when we use perDevPixel > here it overcompensates the scale. That doesn't make sense to me. Does it make sense to you? I suspect that this should be appunits-per-dev-pixel and that changing it to be appunits-per-dev-pixel has exposed another bug.

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #13) > (In reply to Nick Cameron [:nrc] from comment #11) > > I thought so too, but that animates incorrectly - when we use perDevPixel > > here it overcompensates the scale. > > That doesn't make sense to me. Does it make sense to you? It does not make sense to me. > > I suspect that this should be appunits-per-dev-pixel and that changing it to > be appunits-per-dev-pixel has exposed another bug. I will attempt to find it...

Comment on attachment 712819 [details] [diff] [review] patch Review of attachment 712819 [details] [diff] [review]: ----------------------------------------------------------------- Very good! ::: gfx/layers/Layers.h @@ +861,5 @@ > gfxPoint GetFixedPositionAnchor() { return mAnchor; } > Layer* GetMaskLayer() { return mMaskLayer; } > > + // Note that all animation data is in either css pixels or app units > + // and must be converted to device pixels. // Note that all lengths in animation data are either in CSS pixels or app units // and must be converted to device pixels by the compositor. ::: gfx/layers/ipc/CompositorParent.cpp @@ +723,5 @@ > + gfxPoint3D mozOrigin = data.mozOrigin(); > + mozOrigin.x = mozOrigin.x * float(nsDeviceContext::AppUnitsPerCSSPixel()) > + / data.appUnitsPerDevPixel(); > + mozOrigin.y = mozOrigin.y * float(nsDeviceContext::AppUnitsPerCSSPixel()) > + / data.appUnitsPerDevPixel(); Factor out AppUnitsPerCSSPixel/data.appUnitsPerDevPixel into a double and use it repeatedly here.

carrying r=roc

tef? because we need this for OMTA (and thus smooth animations) to work for any resolution other than the default unscaled one.

I'm not sure which release the hidpi devices are tracking, but whichever that is, this needs to block it.

Bumping to leo? since this is not a tef/shira blocker.

Peak is hidpi.

is this going to land in b2g-18 or just in master one? Thx!

Did you mean to clear blocking-b2g?

The patch is very safe and can probably land wherever it's needed. However, I hope we're not planning to add all kinds of high-DPI support to the B2G18 branch. There might be new features needed like high-DPI canvas that wouldn't be suitable for the 18 branch.

(In reply to Ismael from comment #24) > is this going to land in b2g-18 or just in master one? > Thx! Yes, this should land on b2g-18, if that is the right branch for leo+ patches. I was waiting for someone to confirm that. Also, can I have my leo+ back please?

Landing info for 1.1: https://wiki.mozilla.org/Release_Management/B2G_Landing#Landing_leo.2B_bugs_for_v1.1.0_.28updated_2.2F13.29