On Fri, 20 Dec 2019 09:55:18 GMT, Ambarish Rapte <ara...@openjdk.org> wrote:
>> modules/javafx.graphics/src/main/native-glass/mac/GlassView3D.m line 97:
>>
>>> 96: if (pix == NULL)
>>> 97: {
>>> 98: const CGLPixelFormatAttribute attributes2[] =
>>
>> The change looks good.
>> I would suggest to print a message inside the if block, mentioning that the
>> first attempt to create a pixel format object has failed and second attempt
>> is being made with minimal attributes.
>> Also please use `CGLErrorString(err)` when printing the error message.
>
> Additionally the if condition at line 105 can be changed to match with the
> change. as,
> `if (pix == NULL || err != kCGLNoError)`
Could you please be more specific on why you suggest the check be expended with
`|| err != kCGLNoError`?
I see no harm in adding it but it feels like an unnecessary condition to me; my
understanding of the current
[documentation](https://developer.apple.com/library/archive/documentation/GraphicsImaging/Conceptual/OpenGL-MacProgGuide/opengl_pixelformats/opengl_pixelformats.html)
(though the use of the API itself is deprecated by Apple) has me understand
than `pix == NULL` is both a necessary and sufficient test.
-------------
PR: https://git.openjdk.java.net/jfx/pull/65
