buffer too small

Jim Graham james.graham at oracle.com
Wed Mar 9 23:26:00 UTC 2016


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


More information about the openjfx-dev mailing list