RFR: 8310054: ScrollPane insets are incorrect

Phil Race prr at openjdk.org
Thu Jun 15 19:36:09 UTC 2023


On Wed, 14 Jun 2023 20:17:32 GMT, Alexey Ivanov <aivanov at openjdk.org> wrote:

> After the size of `ScrollPane` child component changes, it recalculates the size of the scroll bars and hides or shows them as necessary. This situation is handled in `WScrollPanePeer.childResized`:
> 
> https://github.com/openjdk/jdk/blob/ee4ab6709ebaf8a1b1e9f297a7c53205987f3eba/src/java.desktop/windows/classes/sun/awt/windows/WScrollPanePeer.java#L104-L109
> 
> After [JDK-8297923](https://bugs.openjdk.org/browse/JDK-8297923), `setSpans` is run asynchronously on the toolkit thread. Thus, `setInsets` calculates the incorrect values because `setSpans` hasn't updated the sizes yet.
> 
> This is a regression from [JDK-8297923](https://bugs.openjdk.org/browse/JDK-8297923).
> 
> **Fix**
> 
> The insets are updated in the native implementation of `setSpans` directly.
> 
> The native implementation of `setInsets` method is run on the toolkit thread as a synchronous call because the insets are used in [`WScrollPanePeer.initialize`](https://github.com/openjdk/jdk/blob/ee4ab6709ebaf8a1b1e9f297a7c53205987f3eba/src/java.desktop/windows/classes/sun/awt/windows/WScrollPanePeer.java#L62-L67).

Hmm, I did ask during the previous review if there was any reason to block with
_SetScrollPos or "the case below" - ie  _SetSpans... but perhaps it is more complex than that.

I need some clarification
Before any changes, using SyncCall , _SetSpans was called synchronously.
After the first change to use InvokeFunctionLater,  _SetSpans was called asynchronously .
Now with InvokeFunction,  _SetSpans is again called synchronously

So why do you also need the change to set it in native ?

The difference I can see is that although it is synchronous it now requires jumping on to the Toolkit thread
and so other pending processing on the Toolkit thread will run first.

But I'm not sure that's relevant here.

So can you expand on why you also had to move the call to setInsets() to native ?

-------------

PR Comment: https://git.openjdk.org/jdk/pull/14478#issuecomment-1593613477



More information about the client-libs-dev mailing list