In bug 586683 we clamp the scale factors we use to set resolution to a power of 2, to avoid rerendering too much as the scale factor changes. But if the transform is static we shouldn't clamp, we should use the optimal resolution. This should be a softblocker, it'll be a quality regression over 3.6.

I think the best way to fix this might be to add a new flag on layers "transform is active", only clamp if that flag is set, and set the flag from nsDisplayTransform when the transform is known to be active there. I'm not sure how this should interact with shadowlayers and their transforms though. Should we have them set the flag too, and possibly back out the patch in bug 633040 because it should be fixed by this bug?

The text is all fuzzy. It shouldn't be.

I think the patch in bug 633040 is still correct. BasicManager resolution is for all intents and purposes always an "exact resolution", so I don't think it would affect the "active flag". However we twiddle scales on active/inactive transformed *ShadowableLayers (et al.) doesn't affect *ShadowLayers either because the shadows just draw buffers that have a final computed resolution attached to them, they don't care how the resolution was computed. But maybe I misunderstand something.

Matt, can you work on this? I expect it should be quite easy.

WIP, going to bed.

Clipping needs to happen before we scale the context. This is really ugly, any ideas how we can do this cleanly?

- float paintXRes = BasicManager()->XResolution() * gfxUtils::ClampToScaleFactor(scale.width); - float paintYRes = BasicManager()->YResolution() * gfxUtils::ClampToScaleFactor(scale.height); + float paintXRes = BasicManager()->XResolution() * scale.width; + float paintYRes = BasicManager()->YResolution() * scale.height; + if (GetParent() && GetParent()->IsTransformActive()) { + //Parent (container) has an active transform, clamp to scale factors, otherwise paint at exact resolution. + paintXRes = gfxUtils::ClampToScaleFactor(paintXRes); + paintYRes = gfxUtils::ClampToScaleFactor(paintYRes); Isn't this going to regress 633040 again, because we're going to go back to clamping the resolution * scale instead of just the scale?

Only while the transform component is actively changing! Probably worth fixing though :)

Only clamp the transform component.

I don't understand the clipping changes in the OGL code here. Why are you adding a new clipping call? Why aren't we doing the "transform, round out, clip, inverse transform, round out, clip" dance?

It seems we won't be taking this for desktop FF4.

We already do this to pass a scaled region into BeginUpdate. Though the second clip should be moved outside of the gfxASurface::CONTENT_COLOR_ALPHA if condition.

This should hard block Fennec because it causes corruption for Fennec, which tends to always render at non-identity resolutions.

(In reply to comment #13) > It seems we won't be taking this for desktop FF4. Robert, we'd like to take this for Fennec. Any technical issues or risk with landing it (regarding FF4)?

The smallest fix would be to stop clamping and stop using ExtendToScale with some sort of mobile-only ifdef. That's all we care about.

Fennec needs this so we stop thrashing ashm memory

Comment on attachment 514752 [details] [diff] [review] Part 1: Add IsActiveTransform interface to container layers for fennec

Comment on attachment 515401 [details] [diff] [review] Part 2: BasicLayers can draw at exact resolutions for inactive transforms v3 for fennec

Comment on attachment 514752 [details] [diff] [review] Part 1: Add IsActiveTransform interface to container layers drivers thought an sr is all that is needed

Comment on attachment 515401 [details] [diff] [review] Part 2: BasicLayers can draw at exact resolutions for inactive transforms v3 drivers thought an sr is all that is needed

Comment on attachment 515401 [details] [diff] [review] Part 2: BasicLayers can draw at exact resolutions for inactive transforms v3 No problem taking this for FF4 if Fennec needs it.

This patch breaks some things in Fennec. STR: 1. Go to news.google.com 2. Scroll down to Google Fast Flip 3. Hit the arrows We are busting the assertion that the region to draw bounds be a subset of quadrantRect because we are rounding out the region in the GetContext call. The new region to draw gets passed to BasicLayers.cpp, and when it calls GetContext with the rounded out version which it seems messes up the calculation for what quadrant we are in. Also, moving clipping into GetContext isn't exactly equivalent. We don't apply the clip in BasicLayers.cpp in the content process for non-identity resolutions for better or worse. I've seen that this causes problems with your patch rebased for patches in 635373 (now landed) and 635035. It seems that the regions we are passing back and forth for the buffers aren't working quite properly.

So on, mozilla-central, if I force clipping on in BasicLayers.cpp I see this rendering issue. Hm.

Trying to understand if we still need/want this bug. roc/mattwoodrow can you comment?

On basic layers (where the layer manager has a specified resolution), we don't clamp this value (to avoid the fuzzy text) and end up with non integer/reciprocal values. ExtendForScaling can't actually handle these, and we get assertions and unknown results. So yes, I believe we need this for fennec. These patches were ready to land, but we need to investigate the broken rendering the Ben is seeing.

Removed the now unnecessary call to ExtendForScaling Carrying forward r/sr=roc. I see the "Messed up quadrants" assertion even without this patch, I don't think this is related (the assertion checks the region before we modify it). I can't reproduce any rendering glitches either, but I can't see a "Google Fast Flip" option on news.google.com to test using the STR.

Managed to find Fast flip on the US version of google news, still can't reproduce any corruption different to what I see with tip.

Matt: not sure about tip anymore, but I just now rebased your patches on top of 635035 and there are corruption issues. We could land this now and deal with it in 635035?

If this isn't fixing any fennec bugs I don't see why we should land it. Bug 635035 shouldn't cause corruption (can you be more specific?) but there will be seaming. Are these patches or bug 635035 on their own the cause of the corruption? Bug 637852, which bug 635035 is blocked on, will fix the seaming, and its approach avoids the "ASSERTION: Multiplication factors must be integers or 1/integer" problem for fennec's common case.

Moving discussion to 635035 because I'm seeing the problem without these patches.

This bug seems to fix some rendering issues in about:config.

what is the plan/status here?

This is fixed in my patches in bug 637852.