Last Comment Bug 753784 - Mask layers are applied incorrectly in native android fennec
: Mask layers are applied incorrectly in native android fennec
Status: RESOLVED FIXED
[inbound]
: qawanted
Product: Core
Classification: Components
Component: Graphics: Layers (show other bugs)
: Trunk
: ARM Android
: -- normal (vote)
: mozilla15
Assigned To: Nick Cameron [:nrc]
:
Mentors:
Depends on: 755078
Blocks: 607417
  Show dependency treegraph
 
Reported: 2012-05-10 07:54 PDT by Chris Lord [:cwiiis]
Modified: 2012-06-03 06:56 PDT (History)
13 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
unaffected
+
fixed
-


Attachments
What mw22's page should look like (47.83 KB, image/png)
2012-05-10 12:51 PDT, Naoki Hirata :nhirata (please use needinfo instead of cc)
no flags Details
after rotation from portrait to landscape (43.05 KB, image/png)
2012-05-10 12:52 PDT, Naoki Hirata :nhirata (please use needinfo instead of cc)
no flags Details
patch 1: force a single tile for an image layer (7.29 KB, patch)
2012-05-21 21:32 PDT, Nick Cameron [:nrc]
roc: review+
Details | Diff | Review
Patch 2: expose max texture size API (18.71 KB, patch)
2012-05-21 21:33 PDT, Nick Cameron [:nrc]
roc: review+
Details | Diff | Review
Patch 3: change the mask layer setup (5.66 KB, patch)
2012-05-21 21:34 PDT, Nick Cameron [:nrc]
roc: review+
chrislord.net: feedback+
Details | Diff | Review
Patch 3: change the mask layer setup (5.66 KB, patch)
2012-05-22 14:33 PDT, Nick Cameron [:nrc]
ncameron: review+
Details | Diff | Review
patch 1: force a single tile for an image layer (7.33 KB, patch)
2012-05-22 14:47 PDT, Nick Cameron [:nrc]
ncameron: review+
Details | Diff | Review
Patch 2: expose max texture size API (18.85 KB, patch)
2012-05-22 14:48 PDT, Nick Cameron [:nrc]
ncameron: review+
Details | Diff | Review

Description Chris Lord [:cwiiis] 2012-05-10 07:54:49 PDT
It seems that TiledThebesLayerOGL applies the same mask layer to each tile, instead of to the layer as a whole, resulting in very visible rendering glitches.

See:

http://chrislord.net/files/mozilla/clipping.html
http://people.mozilla.org/~mwargers/tests/layout/white_stripes_scrolling_iframe.html

on native android fennec.
Comment 1 Mark Finkle (:mfinkle) (use needinfo?) 2012-05-10 12:48:10 PDT
Can we get a screenshot of the issue? I'm interested in seeing how bad the problem is and why we don't see it all the time.
Comment 2 Naoki Hirata :nhirata (please use needinfo instead of cc) 2012-05-10 12:51:48 PDT
Created attachment 622863 [details]
What mw22's page should look like
Comment 3 Naoki Hirata :nhirata (please use needinfo instead of cc) 2012-05-10 12:52:46 PDT
Created attachment 622864 [details]
after rotation from portrait to landscape

Going to mw22's test page, and then moving from portrait to landscape, shows a truncation on the right side.
Comment 4 Naoki Hirata :nhirata (please use needinfo instead of cc) 2012-05-10 12:53:17 PDT
Note: Tested with Samsung Galaxy S II ; Nightly 5/10/2012 Multilocale
Comment 5 Benoit Girard (:BenWa) 2012-05-10 12:54:32 PDT
Which two builds did you compare?
Comment 6 Nick Cameron [:nrc] 2012-05-10 15:21:27 PDT
(In reply to Chris Lord [:cwiiis] from comment #0)
> It seems that TiledThebesLayerOGL applies the same mask layer to each tile,
> instead of to the layer as a whole, resulting in very visible rendering
> glitches.
> 
This is intentional, although it might be a bad idea and/or implemented incorrectly

> See:
> 
> http://chrislord.net/files/mozilla/clipping.html
> http://people.mozilla.org/~mwargers/tests/layout/
> white_stripes_scrolling_iframe.html
> 
> on native android fennec.

I'm trying to recreate the glitches in the sceenshot, but can't. I'm looking at these pages in the latest nightly (also a moz-central debug build) native android and tried various combinations of zooming and rotating, but don't get the glitches in the screenshots.

Are there any steps I'm missing? Does tiling have to be enabled on mobile?
Comment 7 Chris Lord [:cwiiis] 2012-05-11 02:41:03 PDT
Sorry, thinking about this with a clearer head, TiledThebesLayerOGL probably isn't at fault alone, as we see the problem occurring on a canvas, which ought to get a CanvasLayerOGL.  I'll look into this further and confirm the root cause.

In reply to comment #6, if you go to the first link in a nightly of native android fennec and zoom in or out, you'll see the second green oblong not match the first (along with some weird rendering artifacts) - Going to the second link on my device always shows the problem, but you may need to zoom in or out, depending on the screen size of your device.

In reply to comment #1, we don't see this everywhere as the majority of pages end up as a single layer, and even when that doesn't happen, setting a border-radius on an element that ends up split out into a layer is pretty rare at the moment.
Comment 8 Chris Lord [:cwiiis] 2012-05-11 03:40:30 PDT
After a bit more testing, it just seems any use of mask layers is broken - I wonder if this is NPOT-texture related? Still investigating.
Comment 9 Chris Lord [:cwiiis] 2012-05-11 05:59:41 PDT
Not found the whole cause yet, but I'm warming to it being OGL layer masks not working when we don't have/aren't using NPOT texture support.

My hunch is that the size returned by ShadowImageLayerOGL is the texture size and it's mismatching with the actual image size, causing the coordinates set in ShaderProgramOGL::LoadMask to be incorrect and reference pixels that just contain junk data (which corresponds to what I see on a device - the mask, squashed, with random areas masked outside of the squashed mask bounds)

While this may not exactly be it, I think it will be something along these lines. Hopefully track it down soon.
Comment 10 Chris Lord [:cwiiis] 2012-05-11 06:48:32 PDT
Found part of the cause - OGL mask layers just don't work when they end up backed by a tiled texture. We wouldn't have seen this on some/a lot of phones because of the GLContext::WantsSmallTiles function - this will return false (and thus most of the time give us big enough textures to hold the mask) when CanUploadSubTextures returns true.

Forcing WantsSmallTiles to false fixes the mask getting squashed on one axis, but it now ends up squashed on both axes, depending on zoom - This would lend further credence to the NPOT texture hypothesis, and would also explain why we don't always see this - both these situations would only turn up on Adreno (GLContext::CanUploadNonPowerOfTwo returns false on Adreno).

This bug should be able to be partially replicated on non-Adreno by setting the pref 'gfx.textures.poweroftwo.force-enabled' to true.

We could fix this by changing how ShaderProgramOGL::LoadMask works and altering all the OGL RenderLayer methods, or I guess by changing how BindAndDrawQuadWithTextureRect works, but I think the former makes more sense seeing as how RenderLayer already has knowledge of tiling.

A band-aid fix would be to fix the NPOT texture problem and just set WantsSmallTiles to false - with BenWa's tiled thebes layers, we no longer rely on TiledTextureImage for tiles. This would increase memory usage, presumably, though.

A more extreme 'fix' would be to roll-back the layer mask support and re-land it when this is fixed.
Comment 11 Chris Lord [:cwiiis] 2012-05-11 08:02:15 PDT
After discussing with BenWa, a fix isn't as simple as I'd hoped...

When using TiledTextureImage, we make no guarantees about the boundaries of tiles overlapping, so there's no quick fix for that. We discussed some possible fixes:

- Render the mask alpha to an FBO and use destination alpha in the blending equation to do masking. Pros, it's pretty easy, cons, it might conflict with that layout want to do in the future, and it involves an intermediate step (so could be slower/involve more memory)

- Introduce a tiled mask layer. BenWa's tiles are always aligned to tile boundaries, so you don't ever end up in the situation that a mask tile could overlap more than one tile. The problem then would be that you can end up in a situation where the mask's valid area doesn't match the masked layer's valid area and you need to read pixels that don't exist. Two possible fixes:
  - Initialise mask layers - could be a problem for opaque layers? May involve an intermediate copy?
  - Take the intersection of the mask and layer tiles' valid regions and use that when drawing. May not be as simple as it sounds?

The quick 'fix' is to #ifdef MOBILE this in layout and make sure that mask layers are always rendered in software. This is slower, but is the situation we were in before accelerated masking landed and will fix glitches. Even with the best of the current set of mobile GPUs, we're going to hit this bug on any layer with a dimension > 2048.
Comment 12 Chris Lord [:cwiiis] 2012-05-11 11:19:42 PDT
I've just spent several hours trying to come up with a patch that disables the mask layers (by partially reverting FrameLayerBuilder and nsGfxScrollFrame), but I've not been successful. Ccing nrc, maybe he has some comment on how this can be easily done.
Comment 13 Joe Drew (not getting mail) 2012-05-11 11:56:57 PDT
Correct me if I'm wrong, but this doesn't apply to aurora (and hence Fennec Native 1), right?
Comment 14 Benoit Girard (:BenWa) 2012-05-11 12:02:32 PDT
The shader masking does not. It would be nice to test the software path used on Native to confirm that it is valid.
Comment 15 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-05-11 15:17:40 PDT
(In reply to Chris Lord [:cwiiis] from comment #11)
> - Render the mask alpha to an FBO and use destination alpha in the blending
> equation to do masking. Pros, it's pretty easy, cons, it might conflict with
> that layout want to do in the future, and it involves an intermediate step
> (so could be slower/involve more memory)

This sounds like the way to go. (Or maybe use a texture instead of an FBO.)
Comment 16 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-05-11 15:19:55 PDT
If the mask image is bigger than texture or FBO size limits, you could clamp the temporary texture/FBO size to some limit and scale down as you render into it, and transform up again when you sample from the texture or render the FBO.
Comment 17 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-05-11 16:06:21 PDT
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #15)
> (In reply to Chris Lord [:cwiiis] from comment #11)
> > - Render the mask alpha to an FBO and use destination alpha in the blending
> > equation to do masking. Pros, it's pretty easy, cons, it might conflict with
> > that layout want to do in the future, and it involves an intermediate step
> > (so could be slower/involve more memory)

As for those "cons": as long as mask-layers work correctly it won't conflict with what layout wants to do in the future. I doubt it would be slower than taking a CPU-based fallback path, since on-CPU clip-to-path is already horrifically slow. It would use more memory (but only temporarily).
Comment 18 Nick Cameron [:nrc] 2012-05-13 04:03:58 PDT
(In reply to Chris Lord [:cwiiis] from comment #12)
> I've just spent several hours trying to come up with a patch that disables
> the mask layers (by partially reverting FrameLayerBuilder and
> nsGfxScrollFrame), but I've not been successful. Ccing nrc, maybe he has
> some comment on how this can be easily done.

This cannot be easily done, sorry. There is no 'software masking', except for the basic layers path. The old way was to do rounded corner rect clipping explicitly, and do it all in software, i.e., forcing a basic layer. The easiest way to do this might be to force basic layers for layers with masks on mobile, but I'm not sure how well this would work in practice.
Comment 19 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-05-13 17:29:08 PDT
(In reply to Chris Lord [:cwiiis] from comment #10)
> Found part of the cause - OGL mask layers just don't work when they end up
> backed by a tiled texture. We wouldn't have seen this on some/a lot of
> phones because of the GLContext::WantsSmallTiles function - this will return
> false (and thus most of the time give us big enough textures to hold the
> mask) when CanUploadSubTextures returns true.

BTW are you talking about some patch that hasn't been checked in yet?
http://mxr.mozilla.org/mozilla-central/search?string=WantsSmallTiles
Comment 20 Chris Lord [:cwiiis] 2012-05-14 04:10:30 PDT
(In reply to Nick Cameron [:nrc] from comment #18)
> (In reply to Chris Lord [:cwiiis] from comment #12)
> > I've just spent several hours trying to come up with a patch that disables
> > the mask layers (by partially reverting FrameLayerBuilder and
> > nsGfxScrollFrame), but I've not been successful. Ccing nrc, maybe he has
> > some comment on how this can be easily done.
> 
> This cannot be easily done, sorry. There is no 'software masking', except
> for the basic layers path. The old way was to do rounded corner rect
> clipping explicitly, and do it all in software, i.e., forcing a basic layer.
> The easiest way to do this might be to force basic layers for layers with
> masks on mobile, but I'm not sure how well this would work in practice.

Right, this is what we want - Mobile works by having a basic layer tree that gets shadowed by a GL layer tree. We want that basic layer tree to use clipping instead of mask layers.

I've been trying to write a patch that does this, but have had very little success so far.
Comment 21 Chris Lord [:cwiiis] 2012-05-14 04:12:08 PDT
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #19)
> (In reply to Chris Lord [:cwiiis] from comment #10)
> > Found part of the cause - OGL mask layers just don't work when they end up
> > backed by a tiled texture. We wouldn't have seen this on some/a lot of
> > phones because of the GLContext::WantsSmallTiles function - this will return
> > false (and thus most of the time give us big enough textures to hold the
> > mask) when CanUploadSubTextures returns true.
> 
> BTW are you talking about some patch that hasn't been checked in yet?
> http://mxr.mozilla.org/mozilla-central/search?string=WantsSmallTiles

No, I'm just talking about central - that link you provide shows what I'm talking about:

http://mxr.mozilla.org/mozilla-central/source/gfx/gl/GLContext.cpp#686
Comment 22 Nick Cameron [:nrc] 2012-05-14 04:22:21 PDT
(In reply to Chris Lord [:cwiiis] from comment #21)
> (In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #19)
> > (In reply to Chris Lord [:cwiiis] from comment #10)
> > > Found part of the cause - OGL mask layers just don't work when they end up
> > > backed by a tiled texture. We wouldn't have seen this on some/a lot of
> > > phones because of the GLContext::WantsSmallTiles function - this will return
> > > false (and thus most of the time give us big enough textures to hold the
> > > mask) when CanUploadSubTextures returns true.
> > 
> > BTW are you talking about some patch that hasn't been checked in yet?
> > http://mxr.mozilla.org/mozilla-central/search?string=WantsSmallTiles
> 
> No, I'm just talking about central - that link you provide shows what I'm
> talking about:
> 
> http://mxr.mozilla.org/mozilla-central/source/gfx/gl/GLContext.cpp#686

But it looks like WantsSmallTiles is only ever called by Thebes layers, so how does it affect mask layers, which are always Image Layers?
Comment 23 Nick Cameron [:nrc] 2012-05-14 04:24:37 PDT
(In reply to Chris Lord [:cwiiis] from comment #20)
> (In reply to Nick Cameron [:nrc] from comment #18)
> > (In reply to Chris Lord [:cwiiis] from comment #12)
> > > I've just spent several hours trying to come up with a patch that disables
> > > the mask layers (by partially reverting FrameLayerBuilder and
> > > nsGfxScrollFrame), but I've not been successful. Ccing nrc, maybe he has
> > > some comment on how this can be easily done.
> > 
> > This cannot be easily done, sorry. There is no 'software masking', except
> > for the basic layers path. The old way was to do rounded corner rect
> > clipping explicitly, and do it all in software, i.e., forcing a basic layer.
> > The easiest way to do this might be to force basic layers for layers with
> > masks on mobile, but I'm not sure how well this would work in practice.
> 
> Right, this is what we want - Mobile works by having a basic layer tree that
> gets shadowed by a GL layer tree. We want that basic layer tree to use
> clipping instead of mask layers.
> 
> I've been trying to write a patch that does this, but have had very little
> success so far.

So you want to do the masking on the drawing thread, rather than the compositing thread, or you want to use software compositing on the compositing thread rather than OpenGL compositing for layers which have masks?

I imagine the former is doable, the latter would be hard.
Comment 24 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-05-14 04:27:25 PDT
(In reply to Chris Lord [:cwiiis] from comment #21)
> (In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #19)
> > BTW are you talking about some patch that hasn't been checked in yet?
> > http://mxr.mozilla.org/mozilla-central/search?string=WantsSmallTiles
> 
> No, I'm just talking about central - that link you provide shows what I'm
> talking about:
> 
> http://mxr.mozilla.org/mozilla-central/source/gfx/gl/GLContext.cpp#686

I swear, this morning that link didn't provide any matches!!!
Comment 25 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-05-14 04:41:20 PDT
OK, so with shadow layers, the mask image is drawn to a memory surface and then ShadowImageLayerOGL::Init calls CreateTextureImage on the surface. Why don't we just make up a new flag in TextureImage::Flags that means "don't tile me", and pass it from ShadowImageLayerOGL::Init? When we change the image in an ImageLayer, we're always replacing the image with a completely different image so I don't think tiling buys us anything here.

If the max texture size bites us, then we can follow comment #16: allocate the biggest texture we can, downscale the image into it, and normalized texture coordinates should upscale for us when we sample it. A reduction in resolution of oversized masks is just fine.
Comment 26 Chris Lord [:cwiiis] 2012-05-14 06:53:21 PDT
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #25)
> OK, so with shadow layers, the mask image is drawn to a memory surface and
> then ShadowImageLayerOGL::Init calls CreateTextureImage on the surface. Why
> don't we just make up a new flag in TextureImage::Flags that means "don't
> tile me", and pass it from ShadowImageLayerOGL::Init? When we change the
> image in an ImageLayer, we're always replacing the image with a completely
> different image so I don't think tiling buys us anything here.
> 
> If the max texture size bites us, then we can follow comment #16: allocate
> the biggest texture we can, downscale the image into it, and normalized
> texture coordinates should upscale for us when we sample it. A reduction in
> resolution of oversized masks is just fine.

This doesn't help us when the texture size doesn't match the requested texture size (as is the case for old Adreno drivers, where using NPOT textures causes crashing) - but I guess we could just scale the mask into whatever surface we happen to get back.

This is a reasonable fix imo, but may not fix ref-tests, due to sampling boundaries (unless we change the ref-tests to only use power-of-two sized, square elements...) - Any thoughts for that situation?
Comment 27 Chris Lord [:cwiiis] 2012-05-14 07:14:53 PDT
(In reply to Nick Cameron [:nrc] from comment #23)
> (In reply to Chris Lord [:cwiiis] from comment #20)
> > (In reply to Nick Cameron [:nrc] from comment #18)
> > > (In reply to Chris Lord [:cwiiis] from comment #12)
> > > > I've just spent several hours trying to come up with a patch that disables
> > > > the mask layers (by partially reverting FrameLayerBuilder and
> > > > nsGfxScrollFrame), but I've not been successful. Ccing nrc, maybe he has
> > > > some comment on how this can be easily done.
> > > 
> > > This cannot be easily done, sorry. There is no 'software masking', except
> > > for the basic layers path. The old way was to do rounded corner rect
> > > clipping explicitly, and do it all in software, i.e., forcing a basic layer.
> > > The easiest way to do this might be to force basic layers for layers with
> > > masks on mobile, but I'm not sure how well this would work in practice.
> > 
> > Right, this is what we want - Mobile works by having a basic layer tree that
> > gets shadowed by a GL layer tree. We want that basic layer tree to use
> > clipping instead of mask layers.
> > 
> > I've been trying to write a patch that does this, but have had very little
> > success so far.
> 
> So you want to do the masking on the drawing thread, rather than the
> compositing thread, or you want to use software compositing on the
> compositing thread rather than OpenGL compositing for layers which have
> masks?
> 
> I imagine the former is doable, the latter would be hard.

I wanted to do the former, cooked up a patch that I thought reverted FrameLayerBuilder/nsGfxScrollFrame to pre-mask-layers state, and I just ended up with incorrect clipping :/
Comment 28 Chris Lord [:cwiiis] 2012-05-14 08:16:45 PDT
Had a look via the inspector, this site is actually a full-screen iframe.

What could be happening here is that the root scrollable frame code is finding the iframe, but the code we use to actually scroll will be trying to scroll the document body node.

In fact, I'm pretty certain this is what's happening - we're scrolling the iframe fine - but the document isn't scrolling because it can't... The compositor picks up the iframe and thinks it did scroll, when in fact, it didn't, so we end up with the scroll applied twice - once from the iframe scrolling, then again by the async scroll code. Or something along these lines at least.

Somehow, we need to have GetPrimaryScrollableLayer not be tricked by a scrollable iframe occupying an entire unscrollable document. I'll know more when this build finishes...
Comment 29 Chris Lord [:cwiiis] 2012-05-14 08:36:21 PDT
Ignore the last comment, wrong bug :|
Comment 30 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-05-14 14:20:55 PDT
(In reply to Chris Lord [:cwiiis] from comment #26)
> This doesn't help us when the texture size doesn't match the requested
> texture size (as is the case for old Adreno drivers, where using NPOT
> textures causes crashing) - but I guess we could just scale the mask into
> whatever surface we happen to get back.

Can we draw the mask image into the texture with EXTEND_PAD and adjust sampling coordinates in the shader so that things are scaled correctly?

If not, stretching the mask image into the texture probably works. I'm sure we can fuzz our way to victory in the reftests. Maybe if we add a no-NPOT flag for use in reftest manifests.
Comment 31 Nick Cameron [:nrc] 2012-05-14 15:43:24 PDT
Bug 755078 is our backup plan - backout mask layers, or at least find out how much effort it is to do that. I will try to work out a medium-term fix here. Two options are to do masking on Android in the drawing thread (except masks on Container layers, I don't know what to do here, because unless we force masks not to tile, there could still be problems with tiled masks) and in software, or force mask layers to always be a single tile and to scale the mask if necessary (I prefer the latter, but performance might be an issue with large masks and software scaling).

Using tiled mask layers is a possible long term fix, but is complicated by the fact that mask tiles and content tiles will not, in general, line up, so we will end up with four times as many draw calls and some pretty complicated logic.
Comment 32 Chris Lord [:cwiiis] 2012-05-18 08:27:25 PDT
(In reply to Nick Cameron [:nrc] from comment #31)
> Bug 755078 is our backup plan - backout mask layers, or at least find out
> how much effort it is to do that. I will try to work out a medium-term fix
> here. Two options are to do masking on Android in the drawing thread (except
> masks on Container layers, I don't know what to do here, because unless we
> force masks not to tile, there could still be problems with tiled masks) and
> in software, or force mask layers to always be a single tile and to scale
> the mask if necessary (I prefer the latter, but performance might be an
> issue with large masks and software scaling).
> 
> Using tiled mask layers is a possible long term fix, but is complicated by
> the fact that mask tiles and content tiles will not, in general, line up, so
> we will end up with four times as many draw calls and some pretty
> complicated logic.

Just to note, tiles always line up with TiledThebesLayerGL - perhaps we should deprecate the general tiling support (which has plenty of bugs along these lines) and add a TiledMaskLayerGL or similar that takes advantage of this?
Comment 33 Nick Cameron [:nrc] 2012-05-18 14:45:20 PDT
But we don't want to rebuild the mask when we scroll, for example, so the mask tiles and the underlying layer tiles will then not line up. Unless I misunderstand your proposal.
Comment 34 Nick Cameron [:nrc] 2012-05-21 21:32:46 PDT
Created attachment 625884 [details] [diff] [review]
patch 1: force a single tile for an image layer
Comment 35 Nick Cameron [:nrc] 2012-05-21 21:33:24 PDT
Created attachment 625885 [details] [diff] [review]
Patch 2: expose max texture size API
Comment 36 Nick Cameron [:nrc] 2012-05-21 21:34:37 PDT
Created attachment 625886 [details] [diff] [review]
Patch 3: change the mask layer setup
Comment 37 Nick Cameron [:nrc] 2012-05-21 21:36:04 PDT
cwiiis: would you be able to try these patches to confirm they fix the problem on your device(s)? I've tested them on my phone, with and without spoofing POT textures and they work. It's on Try now too. Thanks!
Comment 38 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-05-21 22:02:55 PDT
Comment on attachment 625884 [details] [diff] [review]
patch 1: force a single tile for an image layer

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

::: gfx/layers/ImageLayers.h
@@ +653,5 @@
>    virtual nsACString& PrintInfo(nsACString& aTo, const char* aPrefix);
>  
>  
>    nsRefPtr<ImageContainer> mContainer;
> +  bool mForceSingleTile;

Move this down below mScaleMode
Comment 39 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-05-21 22:08:44 PDT
Comment on attachment 625885 [details] [diff] [review]
Patch 2: expose max texture size API

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

::: gfx/layers/ipc/CompositorParent.h
@@ +110,5 @@
>    void SchedulePauseOnCompositorThread();
>    void ScheduleResumeOnCompositorThread(int width, int height);
>  
>  protected:
> +  virtual PLayersParent* AllocPLayers(const LayersBackend& backendType, int* maxTextureSize);

aBackendType, aMaxTextureSize (all over the place in this patch)

::: gfx/layers/ipc/ShadowLayers.h
@@ +358,5 @@
>    Transaction* mTxn;
>    LayersBackend mParentBackend;
>  
>    bool mIsFirstPaint;
> +  PRInt32 mMaxTextureSize;

Move this above mParentBackend
Comment 40 Chris Lord [:cwiiis] 2012-05-22 02:40:48 PDT
Just doing a build with these patches, will report back.
Comment 41 Chris Lord [:cwiiis] 2012-05-22 04:35:41 PDT
Comment on attachment 625886 [details] [diff] [review]
Patch 3: change the mask layer setup

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

Works great, and really nice implementation if I may say so :)
Comment 42 Nick Cameron [:nrc] 2012-05-22 14:31:31 PDT
Great, thanks Chris!
Comment 43 Nick Cameron [:nrc] 2012-05-22 14:31:41 PDT
Try push: https://tbpl.mozilla.org/?tree=Try&rev=becd24002add
Comment 44 Nick Cameron [:nrc] 2012-05-22 14:33:18 PDT
Created attachment 626186 [details] [diff] [review]
Patch 3: change the mask layer setup

Only changed the commit message, carrying r=roc
Comment 45 Nick Cameron [:nrc] 2012-05-22 14:47:18 PDT
Created attachment 626194 [details] [diff] [review]
patch 1: force a single tile for an image layer

Just roc's changes, carrying r=roc
Comment 46 Nick Cameron [:nrc] 2012-05-22 14:48:48 PDT
Created attachment 626195 [details] [diff] [review]
Patch 2: expose max texture size API

fixed comments, carrying r=roc

Note You need to log in before you can comment on or make changes to this bug.