On Wed, 15 Feb 2023 21:55:10 GMT, Kevin Rushforth <k...@openjdk.org> wrote:
>> Thiago Milczarek Sayao has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Improve SWT detection
>
> modules/javafx.graphics/src/main/java/com/sun/glass/ui/gtk/GtkApplication.java
> line 62:
>
>> 60:
>> 61: private static final String GTK2_REMOVED_WARNING =
>> 62: "WARNING: The JavaFX GTK 2 library was removed.";
>
> Maybe add something like: `, so this option will be ignored` to make it more
> clear?
I've inserted it, but opted for a new sentence.
> modules/javafx.graphics/src/main/java/com/sun/glass/ui/gtk/GtkApplication.java
> line 73:
>
>> 71:
>> 72: if (ver != 3) {
>> 73: logger.warning("SWT-GTK uses unsupported major GTK
>> version "
>
> This needs to be turned into a runtime exception, perhaps
> `UnsupportedOperationException`, which is use in other error cases. A quick
> check shows that this is not sufficient for older SWT libraries (e.g., SWT 3,
> which uses GTK 2), since this system property is not set. Changes will also
> be needed in the native code.
Done.
> modules/javafx.graphics/src/main/native-glass/gtk/launcher.c line 128:
>
>> 126: }
>> 127:
>> 128: if (!found) {
>
> I think the detection logic for an already existing version of GTK needs to
> be enhanced now. There are two cases to consider. First, a GTK library is
> loaded that we do support (as of this PR, only GTK 3). Second, a GTK library
> we don't support (which would be GTK 2 for now, and maybe we could add GTK 4
> in the future). Something like this could be added above line 128:
>
>
> if (!found) {
> char *** bad_use_chain = gtk_2_lib_options;
> for (i = 0; bad_use_chain[i] && !found; i++) {
> found = try_libraries_noload(bad_use_chain[i]);
> if (found && gtk_versionDebug) {
> printf("found already loaded GTK library %s\n",
> bad_use_chain[i][1]);
> }
> }
> if (found) {
> return -1;
> }
> }
>
>
> The above code assumes that you restore the removed `gtk2_versioned` and
> `gtk2_not_versioned` arrays and create a `gtk_2_lib_options` that includes
> them.
Inserted it, but changed the output a bit to include "unsupported"
-------------
PR: https://git.openjdk.org/jfx/pull/999
