buffer too small

[Jim Graham]

Hi Johan,

Notes in line below...

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.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.

This is one of the primary reasons why content dimensions and physical 
dimensions are not the same.

> 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.

Where is this code that you referred to above?  Where is the physical 
width passed from/to?  Lines of code?

The capacity checks at the top of updateMaskTexture() are all using 
content dimensions which are appropriate, so where are the capacity 
checks you see which are using the physical width?

> 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());

That may mask the problem, but it doesn't fix the underlying problem. 
newTexWH are supposed to be the new content dimensions and should not be 
based on the old phyiscal dimensions or we will grow the texture 
unnecessarily large in many cases.

> 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.

The buffer is used to update the content portion of the texture, it 
should only ever take the content dimensions of the texture into 
account.  The physical dimensions are not relevant to the discussion here.

There is an optimization that could avoid allocating a new texture that 
could be based on physical dimensions, but you aren't tying into it with 
the above analysis.  I want to get up to speed on what you've found 
above before I can recommend a more appropriate solution...

			...jim