Closed Bug 1539306 Opened 5 years ago Closed 5 years ago

When the URL bar overflows, BasicCompositor adds the URL bar text to the invalid region of every composite

Categories

(Core :: Web Painting, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla68
Tracking Status
firefox-esr60 --- wontfix
firefox66 --- wontfix
firefox67 --- wontfix
firefox68 --- fixed

People

(Reporter: mstange, Assigned: mattwoodrow)

References

(Regressed 1 open bug, Regression)

Details

(Keywords: regression)

Attachments

(3 files, 1 obsolete file)

Steps to reproduce:

  1. Add some debugging code around the call to StartRemoteDrawingInRegion which prints out mInvalidRegion.
  2. Disable WebRender and hardware acceleration.
  3. Go to a web site with a URL that is long enough to overflow the URL bar. Example: data:text/html,________________________________________________________________________________________________________
  4. Move your mouse over toolbarbuttons or select some text in the content.

Expected results:
Only the areas that have changed on the screen should be included in mInvalidRegion.

Actual results:
Two rectangles are added during every composition, even if nothing around the URL bar changed:

 - 407, 78, 798, 56
 - 746, 156, 1148, 56

These rectangles enlarge the region bounds unnecessarily, and they cause us to composite every time a potential composite is triggered, i.e. they prevent us from skipping empty composites.

The layer tree shows that there are two APZ-scrollable painted layers for the URL bar text, both of which have an ancestor mask layer. They are inside a ContainerLayer which also has a mask layer. The full layer dump is attached.
I think the URL bar is APZ-scrollable because we create a display port for the first scrollable scroll frame we encounter in the document.

Here's a function that can print a region:

template<typename RegionT>
void
printf_region(const RegionT& aRegion)
{
  for (auto iter = aRegion.RectIter(); !iter.Done(); iter.Next()) {
    auto r = iter.Get();
    printf(" - %d, %d, %d, %d\n", r.x, r.y, r.width, r.height);
  }
}
Attached file host layer dump (obsolete) —
Attachment #9053777 - Attachment is obsolete: true
Attachment #9053797 - Attachment mime type: application/octet-stream → text/plain

Ah I see, it's a regression from bug 1190117.

ImageLayerProperties::mLastProducerID is initialised using ImageHost::GetLastProducerID(), which only gets set as part of ImageHost::Composite (which isn't called for masks), and always returns -1 here. The comparison happens against ImageHost::GetProducerID(), which returns a valid index, so this always fails, and we always invalidate masks.

The same bug exists for mLastFrameID too.

Blocks: 1190117

I see!

Also, maybe this is a separate bug, but I noticed that the position of the second rectangle is a bit odd. It almost looks like some offset is applied twice. The second rectangle is located way outside the URL bar and invalidates things inside the content area.

Oh, and I found the helpful pref nglayout.debug.widget_update_flashing which is like paint flashing but for BasicCompositor.

Has STR: --- → yes
Keywords: regression
Assignee: nobody → matt.woodrow
Pushed by mwoodrow@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6c20e9cb71b2
Don't transform mask layer invalid regions by the current layer transform, since they are positioned relative to our parent. r=mstange
https://hg.mozilla.org/integration/autoland/rev/2ae5ad0cc2e2
Use the current producer/frame id for mask layers, since they never set the previous one. r=mstange
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
Depends on: 1540128
No longer blocks: 1190117
Regressed by: 1190117
No longer depends on: 1540128
Regressions: 1540128

Noticed some performance improvements! \0/

== Change summary for alert #20414 (as of Thu, 11 Apr 2019 19:57:52 GMT) ==

Improvements:

10% raptor-tp6-slides-firefox osx-10-10-shippable opt 2,187.56 -> 1,965.67
6% raptor-tp6-docs-firefox fcp osx-10-10-shippable opt 1,449.75 -> 1,356.88

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=20414

Regressions: 1546097
Regressions: 1571478
Regressions: 1631509
Has Regression Range: --- → yes
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: