Re: buffer too small

[Jim Graham]

I thought this sounded familiar.  I remember having the thought before, 
but forgot that I actually did something about this already in another 
part of the code.

Both of those sets of methods already exist and they are used in the 
decora ImagePool object to avoid having to allocate a larger scratch 
texture when this over-allocation occurs.  Similar code should be used 
in the updates of the maskTex object.  It would probably be worthwhile 
to immediately adjust it to maxContent dimensions right after it is 
allocated.

That doesn't obviate the fix below, but it adds an optimization...

                        ...jim

On 3/9/16 3:26 PM, Jim Graham wrote:
Hi Johan,

That sounds like the fix then.

Note that there is another optimization issue here that could be
addressed in a follow-on - basically when a larger physical size is
allocated, then we could expand the content dimensions to match what was
allocated.  We would possibly need a new method on the Texture to do
something like "getMaxContentW/H()" and "setContentW/H()", and that
could avoid reallocating a texture in that case...

             ...jim

On 3/9/16 2:01 AM, Johan Vos wrote:
Hi Jim,

Passing the contentWidth to the update() fixes the problem as well, and
if the excessive memory is not needed (as in my proposal), this is of
course much better.
Can I create an issue and suggest the following patch (this is for 8,
but I can do a similar for 9)?

- Johan

---
a/modules/graphics/src/main/java/com/sun/prism/impl/BaseContext.javaThu
Mar 03 03:22:09 2016 +0100

+++
b/modules/graphics/src/main/java/com/sun/prism/impl/BaseContext.javaWed
Mar 09 10:59:35 2016 +0100

@@ -104,7 +104,7 @@

              // since it was bound and unflushed...

              maskTex.update(maskBuffer, maskTex.getPixelFormat(),

                             0, 0, 0, 0, highMaskCol, nextMaskRow,

-                           maskTex.getPhysicalWidth(), true);

+                           maskTex.getContentWidth(), true);

              maskTex.unlock();

              curMaskRow = curMaskCol = nextMaskRow = highMaskCol = 0;

          }


On Tue, Mar 8, 2016 at 10:43 PM, Jim Graham <james.gra...@oracle.com
<mailto:james.gra...@oracle.com>> wrote:

    I think I see the issue.

    In the code that calls maskTex.update(...) it passes
    maskTex.physicalWidth() as the scan stride of the buffer, but the
    scan stride of the buffer is based on the content size, not the
    physical size.  Thus, the checkUpdateParams() method overestimates
    how many bytes are consumed by the operation.

    Changing that to maskTex.getContentWidth() should be fine...

                             ...jim

    On 3/8/16 6:14 AM, Johan Vos wrote:

        We got a number of bug reports (on Android and iOS) reported by
        developers
        using large images:

        java.lang.IllegalArgumentException: Upload requires 2475266
        elements, but
        only 1549938 elements remain in the buffer

        at

com.sun.prism.impl.BaseTexture.checkUpdateParams(BaseTexture.java:354)

        at com.sun.prism.es2.ES2Texture.update(ES2Texture.java:640)

        at

com.sun.prism.impl.BaseContext.flushVertexBuffer(BaseContext.java:106)

        at

com.sun.prism.impl.BaseContext.updateMaskTexture(BaseContext.java:248)

        at
        com.sun.prism.impl.ps

<http://com.sun.prism.impl.ps>.BaseShaderGraphics.renderShape(BaseShaderGraphics.java:482)



        I traced this down to the following:

        initially, a buffer of [1024 * 1024] is allocated by
        BaseContext.validateMaskTexture. When the MaskData becomes
        bigger than 1024
        (w or h), a new buffer is allocated with capacity [newTexW *
        newTexH] with
        newTexW and newTexH the new width/height that are passed when
        creating a
        new Texture. However, the physical size of the texture can be
        different --
        e.g.

        ES2Texture.create (ES2Context, PixelFormat, WrapMode, int, int,
        bool) will
        in some cases set the real texWidth/height to the next power
of 2.

        Subsequently, in next rendering loops when the Texture needs to
        be updated,
        it is checked whether the capacity of the buffer is large enough
        to hold
        the texture. In this case, the physical width is passed and the
        buffer is
        not large enough.

        Adding the following two lines in
        BaseContext.validateMaskTexture() (line
        220) fixes the problem:

                      newTexW = Math.max(newTexW,
        maskTex.getPhysicalWidth());

                      newTexH = Math.max(newTexH,
        maskTex.getPhysicalHeight());

        Using this patch, the size of the buffer will take the physical
        size of the
        texture into account. I'm not sure this is the best approach
though.

        - Johan