If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

position:fixed BasicShadowColorLayers are wrongly translated on scroll

RESOLVED DUPLICATE of bug 668698

Status

()

Core
Layout
RESOLVED DUPLICATE of bug 668698
7 years ago
6 years ago

People

(Reporter: tatiana, Assigned: tatiana)

Tracking

Trunk
x86
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 5 obsolete attachments)

(Assignee)

Description

7 years ago
Created attachment 526694 [details]
dp4.html

scroll dp4.html in remote Fennec

expected outcome:
black square stays on it's position while scrolling

actual outcome:
black square shakes on scroll
(Assignee)

Comment 1

7 years ago
Created attachment 526695 [details] [diff] [review]
reftest
(Assignee)

Updated

7 years ago
Blocks: 607417
(Assignee)

Comment 2

7 years ago
this bug caused by 48f385ad9f0c :
+    const nsIntRect* clipRect = shadow->GetShadowClipRect();
+    if (clipRect) {
+      nsIntRect transformedClipRect(*clipRect);
+      transformedClipRect.MoveBy(shadowTransform._41, shadowTransform._42);
+      shadow->SetShadowClipRect(&transformedClipRect);
+    }

here we apply scale + translate + truncate to int rectangle, 
later in BasicLayerManager::PaintLayer() we translate truncated clipRect back (snapToPixels == true):

      mTarget->Rectangle(gfxRect(clipRect->x, clipRect->y, clipRect->width, clipRect->height), PR_TRUE);
      mTarget->Clip();
(Assignee)

Comment 3

7 years ago
Created attachment 526704 [details] [diff] [review]
Drop extra translations
Assignee: nobody → tanya.meshkova
Attachment #526704 - Flags: review?(roc)
Comment on attachment 526695 [details] [diff] [review]
reftest

Please use hg export to generate your patches. Your patches are showing up in Bugzilla without file names on each hunk.

These tests should not be in reftest-sanity, we should probably move these test-pos-fixed tests to a new directory, say layout/reftests/displayport-fixed-pos.
Comment on attachment 526704 [details] [diff] [review]
Drop extra translations

You'll need a comment in the code explaining why it's correct to apply the transform of the child layer to the child layer's cliprect --- normally, that's wrong.
> These tests should not be in reftest-sanity, we should probably move these
> test-pos-fixed tests to a new directory, say
> layout/reftests/displayport-fixed-pos.

by default remote reftests are running only tests in reftest-sanity directory, what should be done in order to include remote tests from other folders?
I think we started with a small set of tests just to get something working. Joel and Chris can answer more clearly if we have plans to get more things working.
(Assignee)

Comment 8

7 years ago
Created attachment 527291 [details] [diff] [review]
Drop extra translations (v2)
Attachment #526704 - Attachment is obsolete: true
Attachment #526704 - Flags: review?(roc)
Attachment #527291 - Flags: review?(roc)
(Assignee)

Comment 9

7 years ago
Created attachment 527292 [details] [diff] [review]
reftest (v2)
Attachment #526695 - Attachment is obsolete: true
Attachment #527292 - Flags: feedback?(roc)
(Assignee)

Updated

7 years ago
Attachment #527292 - Flags: feedback?(jones.chris.g)
(In reply to comment #7)
> I think we started with a small set of tests just to get something working.
> Joel and Chris can answer more clearly if we have plans to get more things
> working.

Getting all the reftests passing on desktop (again) is on my todo list but not very high atm.  Joel has been working on the tests running on android.
Comment on attachment 527292 [details] [diff] [review]
reftest (v2)

>diff --git a/layout/reftests/displayport-fixed-pos/reftest.list b/layout/reftests/displayport-fixed-pos/reftest.list

If we need to create a new directory for these, let's just put them in
layout/reftests/displayport/.  Rob might have suggestions for existing
directories they fit in.

>diff --git a/layout/tools/reftest/reftest.js b/layout/tools/reftest/reftest.js

>+function RecvSetViewConfig(xsr, ysr, xs, ys)
>+{
>+    enableAsyncScrolling();
>+    rootView().scrollTo(xsr, ysr);
>+    rootView().setScale(xs, ys);
>+}

These settings persist across page loads.  They need to be reset when
the currently-executing test finishes or later tests will fail.
Attachment #527292 - Flags: feedback?(jones.chris.g) → feedback-
Hmm, this patch still doesn't seem right.

I suspect the code that modifies the fixed-pos layer's effective transform should also modify the effective clip rect.
(Assignee)

Comment 13

7 years ago
Created attachment 529482 [details] [diff] [review]
Revert reverse translation
Attachment #527291 - Attachment is obsolete: true
Attachment #527291 - Flags: review?(roc)
Attachment #529482 - Flags: feedback?(roc)
No, that's not what I mean.

You added code to RenderFrameParent that adjusts the fixed-pos shadow layer's effective transform. That same code should also adjust the effective cliprect.
(Assignee)

Comment 15

6 years ago
It's gone after http://hg.mozilla.org/mozilla-central/rev/e302cef502f6 and http://hg.mozilla.org/mozilla-central/rev/b8ea31e6eeb4 fixes
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 668698
(Assignee)

Updated

6 years ago
Attachment #527292 - Attachment is obsolete: true
Attachment #527292 - Flags: feedback?(roc)
(Assignee)

Updated

6 years ago
Attachment #529482 - Attachment is obsolete: true
Attachment #529482 - Flags: feedback?(roc)
You need to log in before you can comment on or make changes to this bug.