RFR: 8271024: Implement macOS Metal Rendering Pipeline [v15]
Ambarish Rapte
arapte at openjdk.org
Mon Aug 4 12:07:10 UTC 2025
On Fri, 1 Aug 2025 21:49:54 GMT, Kevin Rushforth <kcr at 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
More information about the openjfx-dev
mailing list