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.graham at oracle.com
>> <mailto:james.graham at 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
>>
>>