736716 - remove unused and broken tiling code

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Andreas Gal :gal]

The Nexus S doesn't support NPOT textures and we are trying to use tiling, however, I don't think mTileSourceRect is ever initialized.

As a result twidth is 0 here:

        unsigned int twidth = mTileSourceRect.width;
        unsigned int theight = mTileSourceRect.height;

And as a result, this loop here is going to take a _really_ long time:

        // tesselate the visible region with tiles of subrect size                                                                                    
        for (int y = rect.y; y < rect.y + rect.height;  y += theight) {
            for (int x = rect.x; x < rect.x + rect.width; x += twidth) {

Comment 1

[Andreas Gal :gal]

I think this was introduced with bug 628566. I can't find a test so I am not sure this ever worked.

Comment 2

[Andreas Gal :gal]

I have a pretty easy way to reproduce this, happy to test a patch.

Comment 3

[Andreas Gal :gal]

Attachment: patch

Comment 4

[Andreas Gal :gal]

This fixes my test case, but video and webgl are still broken. I assume that has both to do with no NPOT texture support as well but in a different place. If you grant review, can you please also land the patch for me? If there is a better fix, please feel to steal. I don't know this code at all.

Comment 5

[Andreas Gal :gal]

Attachment: patch

Comment 6

[Andreas Gal :gal]

A little explanation: looks like we don't actually use GL_REPEAT anywhere any more, and GLES 2.0 guarantees that we support NPOT textures as long we are in clamp and not GL_REPEAT mode, so we can whole sale nuke this code.

Comment 7

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Comment on attachment 606904 [details] [diff] [review]
patch

A little more explanantion: I added this code to support lightspeed
checkerboard drawing for GL (eventually, I only added basic layer
support).  We removed that in XUL fennec in favor of drawing the CSS
background color.  So this code is unused now.

This would be a really nice optimization for background-repeat, e.g.,
but we can re-add something similar then, when we have the next real
use case in mind.

>diff --git a/gfx/layers/opengl/ImageLayerOGL.cpp b/gfx/layers/opengl/ImageLayerOGL.cpp

>+    // we need to anchor the repeating texture appropriately
>+    // otherwise it will start from the region border instead
>+    // of the layer origin. This is the offset into the texture
>+    // that the region border represents

This comment is confusing.  Remove references to repeating.

>+    float tex_offset_u = (float)(rect.x % iwidth) / iwidth;
>+    float tex_offset_v = (float)(rect.y % iheight) / iheight;

float([blah])

>+    triangleBuffer.addRect(rect.x, rect.y,
>+                           rect.x + rect.width, rect.y + rect.height,
>+                           tex_offset_u, tex_offset_v,
>+                           tex_offset_u + (float)rect.width / (float)iwidth,
>+                           tex_offset_v + (float)rect.height / (float)iheight);

float([blah])

Comment 8

[Andreas Gal :gal]

http://hg.mozilla.org/integration/mozilla-inbound/rev/8191d6cd869d

Comment 9

[Andreas Gal :gal]

Fixed the bad style nit fix:

http://hg.mozilla.org/integration/mozilla-inbound/rev/f797fa3447ac

Comment 10

[Andreas Gal :gal]

Backed out. Time to try server this.

http://hg.mozilla.org/integration/mozilla-inbound/rev/62e6af03fa55

Comment 11

[Mozilla RelEng Bot]

Try run for d7bb632f74fc is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=d7bb632f74fc
Results (out of 8 total builds):
    failure: 8
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-d7bb632f74fc

Comment 12

[Mozilla RelEng Bot]

Try run for d7da069415da is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=d7da069415da
Results (out of 135 total builds):
    success: 111
    warnings: 17
    failure: 7
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-d7da069415da

Comment 13

[Andreas Gal :gal]

Forth time is the charm

http://hg.mozilla.org/integration/mozilla-inbound/rev/2fb48dfe4959

Comment 14

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/8191d6cd869d
https://hg.mozilla.org/mozilla-central/rev/f797fa3447ac
https://hg.mozilla.org/mozilla-central/rev/62e6af03fa55
https://hg.mozilla.org/mozilla-central/rev/2fb48dfe4959