On Thu, 17 Nov 2022 15:42:29 GMT, Johan Vos <[email protected]> wrote:
>> Thiago Milczarek Sayao has refreshed the contents of this pull request, and
>> previous commits have been removed. The incremental views will show
>> differences compared to the previous content of the PR. The pull request
>> contains one new commit since the last revision:
>>
>> 8260528: Clean glass-gtk sizing and positioning code
>
> modules/javafx.graphics/src/main/native-glass/gtk/glass_window.cpp line 713:
>
>> 711:
>> 712: static int geometry_get_window_width(const WindowGeometry
>> *windowGeometry) {
>> 713: return (windowGeometry->final_width.type != BOUNDSTYPE_WINDOW)
>
> minor: wouldn't it be more readable (here and below) if the test was
> reversed? ( `(windowGeometry->final_width.type != BOUNDSTYPE_WINDOW`)
Those functions already existed. Do you prefer to keep as it was or to invert
the `if`?
> modules/javafx.graphics/src/main/native-glass/gtk/glass_window.cpp line 891:
>
>> 889:
>> 890: if (get_frame_extents_property(&top, &left, &bottom, &right)) {
>> 891: if ((top + right + bottom + left) != 0) {
>
> Are we 100% sure the extents can never be negative values?
I'm 99.99% sure :)
> modules/javafx.graphics/src/main/native-glass/gtk/glass_window.cpp line 1047:
>
>> 1045:
>> 1046: if (moved) {
>> 1047: geometry_set_window_x(&geometry, x);
>
> this will set `windowGeometry->refx` where 2 lines below you set
> `windowGeometry.current_x`. Unless there is gravity involved, both refer to
> the same value, one being `int` and the other being `float` though. Is there
> another conceptual difference, or why do we have 2 values here?
I think you're right, I added `current_x/y` to check if a window moved, but
`ref_x/ref_y` might contain the same value (different type).
> modules/javafx.graphics/src/main/native-glass/gtk/glass_window.cpp line 1269:
>
>> 1267: }
>> 1268:
>> 1269: void WindowContextTop::to_front() {
>
> Do you use `raise` and `lower` here because the sibiling parameter is always
> `NULL` ?
Yes. But also `gdk_window_raise` and `gdk_window_lower` seems to better match
the `toFront()` and `toBack()` description.
See:
https://tronche.com/gui/x/xlib/window/XRestackWindows.html
https://tronche.com/gui/x/xlib/window/XRaiseWindow.html
gdk functions calls those Xlib functions internally.
-------------
PR: https://git.openjdk.org/jfx/pull/915
