On Fri, 1 Aug 2025 17:25:56 GMT, Andy Goryachev <ango...@openjdk.org> wrote:
>> Ambarish Rapte has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> kcr: review: crash regression fix
>
> modules/javafx.graphics/src/main/java/com/sun/prism/mtl/MTLFBOTextureData.java
> line 34:
>
>> 32:
>> 33: @Override
>> 34: public void dispose() {
>
> There is something wrong here: this method does not call `super.dispose()`.
> Is it by design (a comment would be nice then).
>
> The `MTLTextureData.dispose()` calls
> `MTLResourceFactory.releaseTexture(pTexture);` which will not be called id
> the context is disposed of.
There is a comment just below (line 39) that explains it. Maybe adding an
explicit "so no need to call super.dispose()" to the comment would be helpful.
> modules/javafx.graphics/src/main/java/com/sun/prism/mtl/MTLRTTextureData.java
> line 40:
>
>> 38: mtlContext.flushVertexBuffer();
>> 39: }
>> 40: super.dispose();
>
> since the call to `super.dispose() `is conditional, are you sure it always
> behaves as expected?
It looks like it's probably OK, but only because the superclass method has the
same check that this one does. If you care relying on that, you will want to
document that assumption, although it might be cleaner to just call
`super.dispose()` unconditionally.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1824#discussion_r2248923369
PR Review Comment: https://git.openjdk.org/jfx/pull/1824#discussion_r2248928150
