Closed Bug 1406183 Opened 7 years ago Closed 7 years ago

Images in fallback content are in the wrong place sometimes

Categories

(Core :: Graphics: WebRender, defect, P1)

Product:
Core
Component:
Graphics: WebRender
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla58
Tracking Flags:
Tracking Status
firefox57 --- unaffected
firefox58 --- unaffected

People

(Reporter: mstange, Assigned: mstange)

References

Details

(Whiteboard: [wr-mvp])

Attachments

(5 files, 1 obsolete file)

Screenshot_20171006_212322.png
341.69 KB, image/png
Details
testcase
605 bytes, text/html
Details
Bug 1406183 - Add a boolean aIsInactiveLayerManager to FrameLayerBuilder so that IsBuildingRetainedLayers can return false for inactive layer managers even if there is no containing PaintedLayerData.
59 bytes, text/x-review-board-request
mattwoodrow
: review+
Details
Bug 1406183 - End empty layer manager dumps with a line break.
59 bytes, text/x-review-board-request
mattwoodrow
: review+
Details
Bug 1406183 - Include basic layer managers in the display list dumps in layers-free mode.
59 bytes, text/x-review-board-request
mattwoodrow
: review+
Details 
      No description provided.
Attached file testcase (obsolete) — 
When hovering the blue box in the testcase, the dark blue square should not move.
With webrender on, the small dark blue image is drawn in the wrong place while the filter is applied. It's probably a BasicImageLayer in the basic layer manager that's created for the fallback nsDisplayFilter.
Whiteboard: [wr-mvp] [triage]
Priority: -- → P3
Whiteboard: [wr-mvp] [triage] → [wr-reserve]
Blocks: stage-wr-trains, layers-free
No longer blocks: stage-wr-next
Whiteboard: [wr-reserve] → [wr-mvp]
Nightly 58 x64 20171006100327 de_DE @ Debian Testing (KDE / Radeon RX480)
1. main profile: gpu process + layers force accel + webrender + webrendest + omtp
2. fresh profile: layers froce accel + webrender + webrendest

Hm, I can't reproduce in both cases. Nothing moves or changes color when hovering. I verified that WebRender is running. And attachment 8915750 [details] looks exactly the same as in 52.4.0 (20170929010941 https://packages.debian.org/de/buster/firefox-esr).
Priority: P3 → P2
Oops! Thanks for checking!

This only happens if you have layers.force-active set to true. I wasn't aware that I had still set this pref.
Attached file testcase — 
Here's a testcase that reproduces the bug even without the force-active pref.
Attachment #8915750 - Attachment is obsolete: true
Assignee: nobody → mstange
Status: NEW → ASSIGNED
This seems to be an existing problem with ImageLayers inside BasicLayerManagers. It doesn't occur without webrender because without webrender, CanOptimizeAwayPaintedLayer returns false because aLayerBuilder->IsBuildingRetainedLayers() returns false, so we don't create an ImageLayer. And IsBuildingRetainedLayers returns false because the layer manager has a mContainingPaintedLayer, which we don't have with layers-free webrender.
Priority: P2 → P1
Comment on attachment 8916171 [details]
Bug 1406183 - Add a boolean aIsInactiveLayerManager to FrameLayerBuilder so that IsBuildingRetainedLayers can return false for inactive layer managers even if there is no containing PaintedLayerData.

https://reviewboard.mozilla.org/r/187420/#review192514
Attachment #8916171 - Flags: review?(matt.woodrow) → review+
Comment on attachment 8916172 [details]
Bug 1406183 - End empty layer manager dumps with a line break.

https://reviewboard.mozilla.org/r/187422/#review192516
Attachment #8916172 - Flags: review?(matt.woodrow) → review+
Comment on attachment 8916173 [details]
Bug 1406183 - Include basic layer managers in the display list dumps in layers-free mode.

https://reviewboard.mozilla.org/r/187424/#review192518
Attachment #8916173 - Flags: review?(matt.woodrow) → review+
Comment on attachment 8916171 [details]
Bug 1406183 - Add a boolean aIsInactiveLayerManager to FrameLayerBuilder so that IsBuildingRetainedLayers can return false for inactive layer managers even if there is no containing PaintedLayerData.

https://reviewboard.mozilla.org/r/187420/#review192520

::: gfx/layers/wr/WebRenderCommandBuilder.cpp:337
(Diff revision 1)
>  
>    context->SetMatrix(context->CurrentMatrix().PreScale(aScale.width, aScale.height).PreTranslate(-aOffset.x, -aOffset.y));
>  
>    switch (aItem->GetType()) {
>    case DisplayItemType::TYPE_MASK:
> +    context->SetMatrix(context->CurrentMatrix().PreScale(aScale.width, aScale.height).PreTranslate(-aOffset.x, -aOffset.y));

oops, this stray change shouldn't be there, will fix before landing
Pushed by mstange@themasta.com:
https://hg.mozilla.org/integration/autoland/rev/62d565a1c99a
Add a boolean aIsInactiveLayerManager to FrameLayerBuilder so that IsBuildingRetainedLayers can return false for inactive layer managers even if there is no containing PaintedLayerData. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/0db7e0090021
End empty layer manager dumps with a line break. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/feb6c81d3a2a
Include basic layer managers in the display list dumps in layers-free mode. r=mattwoodrow
Blocks: 1407507
Flags: qe-verify+
I managed to reproduced this issue using Firefox 58.0a1 (build ID = 20171005220204) on Windows 8.1 x64.
STR:
1. Use testcase: https://bug1406183.bmoattachments.org/attachment.cgi?id=8915750
2. layers.force-active = true
   gfx.webrender.enabled = true
   gfx.webrender.blob-images = true
   gfx.webrendest.enabled = true 
3. Hover over the square.
The dark blue square moves when hovering.
The same thing happens on 58.0b5 build. Am I missing something?
Markus, can you please give me some guidelines?
Thank you.
Flags: needinfo?(mstange)
You're right, the layers.force-active case has not been fixed in this bug. I would even say that that case is WONTFIX - layers.force-active is false by default and we have no plans to turn it on. The bug that was fixed here was the bug with the testcase in comment 4.

So when verifying this bug, please do the following:
1. Use testcase: https://bug1406183.bmoattachments.org/attachment.cgi?id=8916119
2. gfx.webrender.enabled = true
   gfx.webrender.blob-images = true
   gfx.webrendest.enabled = true 
3. Hover over the square.
Flags: needinfo?(mstange)
I can confirm doing steps from comment 24, the square not moves. I verified using Firefox 57.0 and 58.0b5 on Windows 8.1 x64, Ubuntu 14.04 x64 and Mac OS X 10.12.6.
Note: on 58.0b5 gfx.webrendest.enabled preference was removed.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: