RFR: 8304831: TextFlow.hitTest.insertionIndex incorrect with surrogate pairs [v6]

Andy Goryachev angorya at openjdk.org
Mon May 22 15:58:04 UTC 2023


On Mon, 22 May 2023 15:52:55 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:

>>> could you please help me in understanding how `Util.runAndWait(() -> { });` help in this case
>> 
>> After discussing the issue with @kevinrushforth it probably won't.
>> 
>> The idea was to replace Thread.sleep() with something that can work reliably even when the process takes a long time (i.e. cold boot).  Simply increasing the sleep time to 1000 ms may not be sufficient, I think.
>> 
>> What we need is an equivalent of java.awt.Robot.waitForIdle() which we don't have in FX.  The problem here is that all the processing involving CSS, layout may take several pulses, and it certainly not guaranteed to be over when the next event is processed in `Util.runAndWait()`.
>> 
>> We could still use Toolkit.firePulse(), but this apparently is a hack, and it alters the normal control flow - that is something we are trying to avoid.
>> 
>> Another variant is to add something like that to Util and use that in place of Thread.sleep().  This method will trigger and wait for an arbitrary number of pulses (currently 10, but we can pick any reasonable number):
>> 
>> 
>>     /**
>>      * Triggers and waits for 10 pulses to complete in the specified scene.
>>      */
>>     public static void waitForIdle(Scene scene) {
>>         int count = 10;
>>         CountDownLatch latch = new CountDownLatch(count);
>>         Runnable pulseListener = () -> {
>>             latch.countDown();
>>             Platform.requestNextPulse();
>>         };
>> 
>>         runAndWait(() -> {
>>             scene.addPostLayoutPulseListener(pulseListener);
>>         });
>> 
>>         try {
>>             Platform.requestNextPulse();
>>             waitForLatch(latch, 30, "waitForIdle timeout");
>>         } finally {
>>             runAndWait(() -> {
>>                 scene.removePostLayoutPulseListener(pulseListener);
>>             });
>>         }
>>     }
>> 
>> 
>> I am not sure why we have Scene.addPulseListener() and not a static equivalent of Robot.waitForIdle(), but here is some earlier work on the pulse listener:
>> 
>> https://bugs.openjdk.org/browse/JDK-8097917
>
> @andy-goryachev-oracle Do you think this needs a second reviewer, or are you satisfied that a single review is sufficient?

I don't mind someone else take a look at this, @kevinrushforth .

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

PR Comment: https://git.openjdk.org/jfx/pull/1091#issuecomment-1557477810


More information about the openjfx-dev mailing list