Closed Bug 1209100 Opened 4 years ago Closed 4 years ago

Section headers of ecma-262 doc scroll differently from body text

Categories

(Core :: Graphics: Layers, defect)

44 Branch
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
firefox44 --- wontfix
firefox45 - wontfix
firefox46 + fixed
firefox47 + fixed
firefox48 --- fixed
firefox-esr45 - wontfix

People

(Reporter: Swatinem, Assigned: mstange)

References

()

Details

(Keywords: regression)

Attachments

(4 files)

On linux (arch with latest updates and recent nightly) with APZ ON, smoothScroll OFF, scrolling the ECMA-262 standards doc *with a touchpad* leads to the following behavior:

The section headers scroll like they should but the body text in between stays where it is and just occasionally jumps. Also, some smearing effect can be seen at the top-most section header.

The effect is more obvious when slowly scrolling the doc, it is less obvious when scrolling through it fast. It is also not observable with a mousewheel or when dragging the scrollbar.
It is however observable when using autoscroll (middle mouse button), where you can also adjust the speed so the effect is super obvious.

I captured a video of the effect here: https://www.youtube.com/watch?v=NNPPDndyvjU (sorry for the bad quality)
I can't reproduce this on OS X.
Can you confirm that this does *not* happen if APZ is disabled? It's surprising to me that you're seeing this with autoscroll because that codepath doesn't use APZ at all.
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #2)
> Can you confirm that this does *not* happen if APZ is disabled? It's
> surprising to me that you're seeing this with autoscroll because that
> codepath doesn't use APZ at all.

Hah, you are absolutely right. I can also reproduce this with apz disabled.

Some other things that might help:
Device ID	Mesa DRI Intel(R) Sandybridge Mobile
Driver Version	3.0 Mesa 10.6.7
GPU Accelerated Windows	0/1 Basic (OMTC) 
AzureContentBackend	cairo
CairoUseXRender	1
Summary: Section headers of ecma-262 doc scroll differently from body text with apz enabled → Section headers of ecma-262 doc scroll differently from body text
This may be a recent regression; do you mind trying on Beta or Release and if you don't see it there, tracking down a regression window? Thanks!
No longer blocks: all-aboard-apz
OS: Unspecified → Linux
Version: unspecified → 44 Branch
Pushlog version of that: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a38c23ccca0a&tochange=be81b8d6fae9

Nothing in that range seems like an obvious cause, so we'll probably need to test inbound builds to narrow down the range.
Attached image invalidation.png
With the new ECMAScript doc (http://tc39.github.io/ecma262/#sec-json-object) I have a similar invalidation problem that might be related to this one.

The scrolling is not smooth but jumpy, and after scrolling, the hit rectangles (for example for hover over links) are off by a few pixels. When I click on an identifier to highlight all occurrences, I get what is captured in the attached screenshot: the highlighed identifiers are drawn over the old page, but all are offset by some pixels (the same amount as the mouse hover).
Also, scrolling again corrects at least the broken drawing, but when the page comes to rest after scroll, the hit regions for hover are offset again, and toggling an identifier triggers the same problem all over again.
As suggested by Markus, I tried this with and without opengl acceleration.
With acceleration forced to on (GPU Accelerated Windows	1/1 OpenGL (OMTC)), the invalidation problem does *not* occur. It does only with basic compositing.
Duplicate of this bug: 1238909
The jumpy scrolling of the main PaintedLayer happens because our layer tree invalidation code thinks its transform hasn't changed, so we don't recomposite those areas of the window.
This page is very long, and the PaintedLayer's y translation is about -1.55962e+06 on my machine. In LayerTreeInvalidation.cpp we call FuzzyEqualsMultiplicative on the layer's transform, which returns true even if the translation has changed by several pixels.
I think we need to use a smaller epsilon in FuzzyEqualsMultiplicative, ideally without regressing bug 1165185 for certain transforms.
This is a regression from bug 1165185.
Blocks: 1165185
Component: Panning and Zooming → Graphics: Layers
Keywords: regression
OS: Linux → All
Hardware: Unspecified → All
STR for me (I think it's the same bug):

1. Open http://heycam.github.io/webidl/.
2. Press End to scroll to the end of the document.
3. Use the mouse wheel to scroll up one notch.
4. Mouse over the "double" or "float" link and notice being repainted a few pixels off from where it should be.
Duplicate of this bug: 1248317
[Tracking Requested - why for this release]: There is a possibility that you click the unintended link.

please backed out the bunch of patch bug 1165185 from BETA 45 (next ESR) tree.
Should we be using FuzzyEqualsAdditive there instead? We care about the delta between the two transforms regardless of the magnitude of the values, I think.
bug 1165185 landed a while ago. I don't think we can just backout the patch...
Markus, any chance you will be fixing this?
I was away for two weeks, and this is at the top of my list now.
Flags: needinfo?(mstange)
Tracking for 46+ since we aim to keep apz enabled by default. 
Regression with APZ from the comments in the duplicate bug 1248317.
Assigning this to Markus from comment 18 also (since I'm wontfixing unassigned bugs right now)
Assignee: nobody → mstange
Attached video bug.ogg
[Tracking Requested - why for this release]: Renering error, There is a possibility that you click the unintended link.

(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #19)
> Tracking for 46+ since we aim to keep apz enabled by default. 
> Regression with APZ from the comments in the duplicate bug 1248317.

This problem happens regardless with APZ on/off.

Anyway, It is time to back out for 46 before 46 becomes beta.
And also for ESR45.
I'll take care of that today.
Markus, any news? Thanks
Flags: needinfo?(mstange)
jet -- can you provide an update. Thanks!
Flags: needinfo?(bugs)
I couldn't think of a good short term solution. I'll back out bug 1165185. This will cause more invalidations (and slower scrolling) when scrolling over SVGs, which commonly have strange transforms set on them. But at least we'll properly recomposite the bottom end of very long websites.

I started three try pushes for the backout on the different branches:

inbound: https://treeherder.mozilla.org/#/jobs?repo=try&revision=77a06c168a2f
aurora: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0ca398cc805e
beta: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3e0637d2ee3b
Flags: needinfo?(mstange)
Flags: needinfo?(bugs)
https://hg.mozilla.org/mozilla-central/rev/d9667017bada
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Duplicate of this bug: 1255962
Duplicate of this bug: 1202714
Duplicate of this bug: 1260034
Blocks: 1260034
Markus, should this back out of 46 and 47 as well?
Flags: needinfo?(mstange)
Yes. I'll attach the backout patches rebased to aurora and beta and request uplift.
Flags: needinfo?(mstange)
Approval Request Comment
[Feature/regressing bug #]: regression from bug 1165185, this patch backs the offending patch out
[User impact if declined]: not recompositing properly during scrolling at the bottom of very long web pages
[Describe test coverage new/current, TreeHerder]: none
[Risks and why]: extremely low, just restores previous behavior
[String/UUID change made/needed]: none
Attachment #8736926 - Flags: approval-mozilla-aurora?
Approval Request Comment: see previous comment
Attachment #8736928 - Flags: approval-mozilla-beta?
Comment on attachment 8736926 [details] [diff] [review]
backout patch for aurora

OK to backout of aurora.
Attachment #8736926 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment on attachment 8736928 [details] [diff] [review]
backout patch for beta

Backout for beta to fix a scrolling regression
Attachment #8736928 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.