Content scaling via CSS transform is jagged while layers are active

RESOLVED FIXED

Status

()

Firefox for Android
General
P2
normal
RESOLVED FIXED
6 years ago
4 years ago

People

(Reporter: nhirata, Assigned: cpeterson)

Tracking

unspecified
ARM
Android
Points:
---

Firefox Tracking Flags

(firefox11 fixed, fennec11+)

Details

(Whiteboard: [inbound])

Attachments

(1 attachment, 1 obsolete attachment)

1. go to ftp.mozilla.org/pub/mozilla.org/mobile
2. pan

Expected: the letters/words should not get pixalated while panning
Actual: the letters temporarily get pixalated once the panning movement stops and then snaps to being antialiased.
occurs on HTC Flyer : 20111130 build
Assignee: nobody → chrislord.net
Priority: -- → P2
Summary: panning around on a ftp site causes the letters to temporarily be pixalated. → Content scaling via CSS transform is jagged while layers are active
Simple STR: Go to any site and pan around. You will see the text become jagged and then look nice a few ms later.

The jagged invalidation is triggered by the setting of the transform, and the nice-looking invalidation is triggered by the layers becoming inactive 100 ms afterward:

#0  0x728eac40 in nsWindow::Invalidate(nsIntRect const&, bool) ()
   from /Users/pwalton/Source/moz/birch/objdir-ff-android/dist/bin/libxul.so
...
   from /Users/pwalton/Source/moz/birch/objdir-ff-android/dist/bin/libxul.so
#16 0x723684f4 in nsIFrame::InvalidateFrameSubtree() ()
   from /Users/pwalton/Source/moz/birch/objdir-ff-android/dist/bin/libxul.so
#17 0x7236cc94 in LayerActivityTracker::NotifyExpired(LayerActivity*) ()
   from /Users/pwalton/Source/moz/birch/objdir-ff-android/dist/bin/libxul.so
#18 0x72367226 in nsExpirationTracker<LayerActivity, 4u>::AgeOneGeneration() ()
   from /Users/pwalton/Source/moz/birch/objdir-ff-android/dist/bin/libxul.so
#19 0x72367cf4 in nsExpirationTracker<LayerActivity, 4u>::TimerCallback(nsITimer
roc or Matt, do you have any idea what could be causing this? Zooming in shows that the text does not seem to be antialiased properly during the jagged paint.

We have a CSS scale transform and a translation placed on the browser element. The translation is always to pixel boundaries. Temporarily removing the scale transform fixes the issue, so this might have something to do with non-integer transforms.
According to the output from PrintDisplayList(), the display lists are identical.
Added some logging to nsTextFrame::PaintText(). Although the frame origins are identical between the ugly and nice text runs, the text baseline points (as computed by nsLayoutUtils::GetSnappedBaselineY()) are different. For the ugly paints they're snapped to pixel boundaries, so we have e.g. a baseline point like (2940,3600) for frame origin (2940,1327). For the nice paints the baseline point for the same frame origin is (2940,3552.5).
I believe this is because the Cairo matrix for the ugly paints is [ 0.5 0 ] [ 0 0.5 ], while the Cairo matrix for the nice paints is [ 0.489796 0 ] [ 0 0.489796 ], as I would expect. The scale that has been set is 0.489796.
One of the container layers here in the ugly case has the matrix [ 0.979592 0 ] [ 0 0.979592 ], which may be relevant since 0.5 = 0.489796 / 0.979592.
This may simply be a matter of needing to make ChooseScaleAndSetTransform smarter in FrameLayerBuilder.cpp.

We're doing this:
    //Scale factors are normalized to a power of 2 to reduce the number of resolution changes
    scale = transform2d.ScaleFactors(true);
    // For frames with a changing transform that's not just a translation,
    // round scale factors up to nearest power-of-2 boundary so that we don't
    // keep having to redraw the content as it scales up and down. Rounding up to nearest
    // power-of-2 boundary ensures we never scale up, only down --- avoiding
    // jaggies. It also ensures we never scale down by more than a factor of 2,
    // avoiding bad downscaling quality.
    gfxMatrix frameTransform;
    if (aContainerFrame->AreLayersMarkedActive(nsChangeHint_UpdateTransformLayer) &&
        aTransform &&
        (!aTransform->Is2D(&frameTransform) || frameTransform.HasNonTranslationOrFlip())) {
      scale.width = gfxUtils::ClampToScaleFactor(scale.width);
      scale.height = gfxUtils::ClampToScaleFactor(scale.height);
    } else {

It sounds like we need to not clamp the scale if the new scale matches the old scale of the layer, or something like that.
Found that just before you posted :) I can confirm that this issue is resolved when commenting out that code.

This issue also occurs right after zooming in, in which case the new scale won't match the old scale, but we still want to render without jaggies. So I'm not sure what to do there. In the limit maybe we could just #ifdef this logic out when MOZ_JAVA_COMPOSITOR is on. Unless you have a better solution, of course.
(In reply to Patrick Walton (:pcwalton) from comment #9)
> This issue also occurs right after zooming in, in which case the new scale
> won't match the old scale, but we still want to render without jaggies. So
> I'm not sure what to do there. In the limit maybe we could just #ifdef this
> logic out when MOZ_JAVA_COMPOSITOR is on. Unless you have a better solution,
> of course.

We really don't want to comment this out for you since that will mean that during zooming, we'll constantly re-render the content at every new zoom factor, which will make for slow zooming. It will also affect the handling of CSS transforms in Web content.

What we can do:
1) don't clamp the scale when the new desired scale matches the old desired scale. You can read the current transform on aLayer to check that.
2) Have some way of immediately marking the transform is inactive when you end a zoom or pan.
Those both sound good to me.
Duplicate of this bug: 707878
Created attachment 579652 [details] [diff] [review]
Fix unnecessary scale clamping

I only just realised this was assigned to me, so here's a patch that implements suggestion 1, as I've understood from the comments.
Attachment #579652 - Flags: review?(roc)
Attachment #579652 - Flags: feedback?(pwalton)
Just a note, I got feedback+ on the addition of a != operator on gfx3DMatrix from Matt Woodrow.
Created attachment 579653 [details] [diff] [review]
Fix unnecessary scale clamping

Erk, and of course the patch doesn't even use it (remnants from an earlier revision) - sorry for churn, here's the patch with that part removed.
Attachment #579652 - Attachment is obsolete: true
Attachment #579652 - Flags: review?(roc)
Attachment #579652 - Flags: feedback?(pwalton)
Attachment #579653 - Flags: review?(roc)
Attachment #579653 - Flags: feedback?(pwalton)
Comment on attachment 579653 [details] [diff] [review]
Fix unnecessary scale clamping

Review of attachment 579653 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/base/FrameLayerBuilder.cpp
@@ +1705,5 @@
> +      gfxMatrix oldFrameTransform2d;
> +      if (aLayer->GetTransform().Is2D(&oldFrameTransform2d)) {
> +        gfxSize oldScale = oldFrameTransform2d.ScaleFactors(true);
> +        if (oldScale == scale || oldScale == gfxSize(1.0, 1.0))
> +          clamp = false;

{} around the if body
Attachment #579653 - Flags: review?(roc) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/1e0a2b58599c
Whiteboard: [inbound]
https://hg.mozilla.org/mozilla-central/rev/1e0a2b58599c
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Duplicate of this bug: 707790
Duplicate of this bug: 708287
Duplicate of this bug: 708749
Comment on attachment 579653 [details] [diff] [review]
Fix unnecessary scale clamping

f+ to get this out of my queue.
Attachment #579653 - Flags: feedback?(pwalton) → feedback+
tracking-fennec: --- → 11+
status-firefox11: --- → fixed
I continue to see something like this in nightly.  Patrick, could this have regressed?
It was never actually fixed, in my experience. I think we need option (2) above.
Let's reopen this then. I'm unasssigning from myself for now - please feel free to take it, but I will if I have time after dealing with other issues.
Assignee: chrislord.net → nobody
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
I downloaded the latest nightly and tried it on my phone using the steps in comment #0. I don't seem to see the bug.
(Assignee)

Updated

5 years ago
Assignee: nobody → cpeterson

Updated

5 years ago
Duplicate of this bug: 721790
Marking as fixed; with maple there is nothing more to do here.
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago5 years ago
Resolution: --- → FIXED

Updated

4 years ago
Depends on: 870192
You need to log in before you can comment on or make changes to this bug.