650759 - position:fixed BasicShadowColorLayers are wrongly translated on scroll

Status: RESOLVED DUPLICATE of bug 668698

(Core :: Layout, defect)

Description

[Tatiana Meshkova (:tatiana)]

Attachment: 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

Comment 1

[Tatiana Meshkova (:tatiana)]

Attachment: reftest

Comment 2

[Tatiana Meshkova (:tatiana)]

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();

Comment 3

[Tatiana Meshkova (:tatiana)]

Attachment: Drop extra translations

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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.

Comment 6

[Oleg Romashin (:romaxa)]

> 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?

Comment 7

[(not currently active) Ted Mielczarek]

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.

Comment 8

[Tatiana Meshkova (:tatiana)]

Attachment: Drop extra translations (v2)

Comment 9

[Tatiana Meshkova (:tatiana)]

Attachment: reftest (v2)

Comment 10

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

(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 11

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

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.

Comment 12

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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.

Comment 13

[Tatiana Meshkova (:tatiana)]

Attachment: Revert reverse translation

Comment 14

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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.

Comment 15

[Tatiana Meshkova (:tatiana)]

It's gone after http://hg.mozilla.org/mozilla-central/rev/e302cef502f6 and http://hg.mozilla.org/mozilla-central/rev/b8ea31e6eeb4 fixes