728625 - Handle mNeedsYFlip case in CanvasLayerOGL::RenderLayer with POT-texture forcing

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[George Wright (:gw280) (needinfo me!)]

The patch that will land with bug 721489 needs to handle mNeedsYFlip in CanvasLayerOGL::RenderLayer.

Comment 2

[Joe Drew (not getting mail)]

Attachment: Support flipping quads with texture rects

Comment 3

[George Wright (:gw280) (needinfo me!)]

Comment on attachment 601382 [details] [diff] [review]
Support flipping quads with texture rects

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

LGTM; just parameter naming seems off but everything else looks fine. r+

::: gfx/gl/GLContext.cpp
@@ +2524,5 @@
>  void
>  GLContext::DecomposeIntoNoRepeatTriangles(const nsIntRect& aTexCoordRect,
>                                            const nsIntSize& aTexSize,
> +                                          RectTriangles& aRects,
> +                                          bool flip_y /* = false */)

flip_y seems to be named using a different convention to all the other parameters

::: gfx/gl/GLContext.h
@@ +1383,5 @@
>       */
>      static void DecomposeIntoNoRepeatTriangles(const nsIntRect& aTexCoordRect,
>                                                 const nsIntSize& aTexSize,
> +                                               RectTriangles& aRects,
> +                                               bool flip_y = false);

Same here

Comment 4

[Benoit Jacob [:bjacob] (mostly away)]

Comment on attachment 601382 [details] [diff] [review]
Support flipping quads with texture rects

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

::: gfx/gl/GLContext.cpp
@@ +2502,5 @@
> +        texCoords.AppendElement(t);
> +        t.u = tx1; t.v = ty0;
> +        texCoords.AppendElement(t);
> +        t.u = tx1; t.v = ty1;
> +        texCoords.AppendElement(t);

I didn't know we were doing all these AppendElement calls. Is this performance critical? How many elements can we have in this array? Notice that nsTArray's growth behavior ceases being strict doubling above page size, and instead resizes to the next multiple of page size, so, asymptotically, we have O(n^2) complexity here where n = array size in bytes. That behavior only starts at n >= 12k ( = 3 * page size ), though. Since sizeof(vert_coord)==8, and there are 6 coords per rect, that means that sub-optimal behavior starts at

  (12 * 1 024) / (6 * 8) = 256

rectangles. That seems quite realistic (e.g. a 2d game using hundreds of DOM images)

Comment 5

[Joe Drew (not getting mail)]

It's unlikely but possible. The DOM images would need to be layerized for this code to even be hit more than once. Still, we could add capacity for the 6 elements we know we're going to add every time this code gets hit.

Comment 6

[Joe Drew (not getting mail)]

Oh:
         nsAutoTArray<vert_coord, 6> vertexCoords;
         nsAutoTArray<tex_coord, 6>  texCoords;

These are _already_ preallocated to store 6 elements.

Comment 7

[Joe Drew (not getting mail)]

https://hg.mozilla.org/projects/maple/rev/92afa414d262

Comment 8

[Benoit Jacob [:bjacob] (mostly away)]

Oh, I didn't realize that these were going into separate arrays everytime. I was afraid that would get mungled into a single big array. ok.