RFR: 8271024: Implement macOS Metal Rendering Pipeline [v15]
Kevin Rushforth
kcr at openjdk.org
Fri Aug 1 22:49:09 UTC 2025
On Fri, 1 Aug 2025 17:25:56 GMT, Andy Goryachev <angorya at 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
More information about the openjfx-dev
mailing list