buffer too small
Kevin Rushforth
kevin.rushforth at oracle.com
Wed Mar 9 15:59:00 UTC 2016
Yes, please. Just get Jim to code review it and you can push it to 9-dev
yourself, since you are a committer.
For JDK 8, you need to request approval to backport. If approved by me,
then you can push it to 8u-dev.
-- Kevin
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.java Thu
> Mar 03 03:22:09 2016 +0100
>
> +++ b/modules/graphics/src/main/java/com/sun/prism/impl/BaseContext.java Wed
> 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> 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
>>> .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