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