Closed Bug 728625 Opened 8 years ago Closed 8 years ago

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

Categories

(Core :: Graphics, defect, minor)

x86
Android
defect
Not set
minor

Tracking

()

RESOLVED FIXED

People

(Reporter: gw280, Assigned: joe)

References

Details

(Whiteboard: maple)

Attachments

(1 file)

The patch that will land with bug 721489 needs to handle mNeedsYFlip in CanvasLayerOGL::RenderLayer.
Duplicate of this bug: 730106
Assignee: gwright → joe
Status: NEW → ASSIGNED
Whiteboard: maple
Attachment #601382 - Flags: review?(gwright)
Attachment #601382 - Flags: review?(bjacob)
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
Attachment #601382 - Flags: review?(gwright) → review+
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)
Attachment #601382 - Flags: review?(bjacob) → review+
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.
Oh:
         nsAutoTArray<vert_coord, 6> vertexCoords;
         nsAutoTArray<tex_coord, 6>  texCoords;

These are _already_ preallocated to store 6 elements.
https://hg.mozilla.org/projects/maple/rev/92afa414d262
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
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.
You need to log in before you can comment on or make changes to this bug.