On Fri, 1 Aug 2025 21:49:54 GMT, Kevin Rushforth <k...@openjdk.org> wrote:
>> 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.
Appended the comment as suggested.
>> 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.
Changed to call `super.dispose()` unconditionally. It is safe as
`super.dispose()` method also has same check.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1824#discussion_r2251256000
PR Review Comment: https://git.openjdk.org/jfx/pull/1824#discussion_r2251265785
