On Tue, 8 Oct 2024 13:22:42 GMT, Ambarish Rapte <ara...@openjdk.org> wrote:
>> Thiago Milczarek Sayao has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Let `createGraphics` fail if called after dispose > > modules/javafx.graphics/src/main/java/com/sun/prism/es2/IOSGLDrawable.java > line 32: > >> 30: >> 31: private static native long nCreateDrawable(long nativeWindow, long >> nativeCtxInfo); >> 32: private static native void nDestroyDrawable(long nativeCtxInfo); > > I would recommend to rename these method as `nReleaseDrawable`. > I see we use names as ReleaseXxxx and DeleteXxxx. The DestroyXxxx name would > be a new addition, I think we could avoid the variation. Renamed as suggested. > modules/javafx.graphics/src/main/native-prism-es2/ios/IOSGLDrawable.c line 55: > >> 53: >> 54: /* initialize the structure */ >> 55: memset(dInfo, 0, sizeof(DrawableInfo)); > > Minor: The declaration of `initializeDrawableInfo` from line#35 should be > removed. I've forgot it. Removed. > modules/javafx.graphics/src/main/native-prism-es2/ios/IOSGLDrawable.c line > 126: > >> 124: >> 125: free(dInfo); >> 126: } > > Minor: Please add a new line here. Added. > modules/javafx.graphics/src/main/native-prism-es2/macosx/MacGLDrawable.c line > 120: > >> 118: >> 119: free(dInfo); >> 120: } > > Minor: Please add a new line here. Added > modules/javafx.graphics/src/main/native-prism-es2/windows/WinGLDrawable.c > line 76: > >> 74: >> 75: /* initialize the structure */ >> 76: memset(dInfo, 0, sizeof(DrawableInfo)); > > Minor: The declaration of `initializeDrawableInfo` from line#38 should be > removed. I've forgot it. Removed. > modules/javafx.graphics/src/main/native-prism-es2/windows/WinGLDrawable.c > line 148: > >> 146: >> 147: free(dInfo); >> 148: } > > Minor: Please add a new line here. Added > modules/javafx.graphics/src/main/native-prism-es2/x11/X11GLDrawable.c line 36: > >> 34: #include "com_sun_prism_es2_X11GLDrawable.h" >> 35: >> 36: extern void initializeDrawableInfo(DrawableInfo *dInfo); > > Minor: The declaration of function `initializeDrawableInfo` should be removed > as well. I've forgot it. Removed. ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1586#discussion_r1792039652 PR Review Comment: https://git.openjdk.org/jfx/pull/1586#discussion_r1792038878 PR Review Comment: https://git.openjdk.org/jfx/pull/1586#discussion_r1792037269 PR Review Comment: https://git.openjdk.org/jfx/pull/1586#discussion_r1792039091 PR Review Comment: https://git.openjdk.org/jfx/pull/1586#discussion_r1792038660 PR Review Comment: https://git.openjdk.org/jfx/pull/1586#discussion_r1792037511 PR Review Comment: https://git.openjdk.org/jfx/pull/1586#discussion_r1792038383
