Bug 400727

Summary: StyledText#addBidiSegmentListener(..) only works if RTL input language is installed
Product: [Eclipse Project] Platform Reporter: Markus Keller <markus.kell.r>
Component: SWTAssignee: Markus Keller <markus.kell.r>
Status: RESOLVED FIXED QA Contact:
Severity: major    
Priority: P3 CC: daniel_megert, Lina.Kemmel, markus.kell.r, robin, Silenio_Quarti, tomerm
Version: 4.3Keywords: api
Target Milestone: 4.3 M6   
Hardware: PC   
OS: Windows 7   
Whiteboard:
Bug Depends on:    
Bug Blocks: 400662    
Attachments:
Description Flags
Fix none

Description Markus Keller CLA 2013-02-13 14:21:38 EST 
On Windows, StyledText#addBidiSegmentListener(..) only works if an RTL input language is installed in the OS.

This limitation is not documented there, and it is unexpected. Javadocs of BidiSegmentEvent and BidiSegmentListener#lineGetSegments(BidiSegmentEvent) mention "in a bidi locale", but that is not the correct precondition, and is easy to miss (especially on non-Windows platforms, where bidi-processing is always enabled, see StyledText#isBidi()).

Furthermore, org.eclipse.swt.internal.BidiUtil.isBidiPlatform() caches the value, so changes in the OS are not reflected until the application is restarted.

This hits us in bug 400662 (and in debug's ProcessPropertyPage).

I think the best solution is to remove StyledText#isBidi() and always process BidiSegmentListeners. Eclipse workbench plug-ins that want to reduce bidi-processing overhead should rely on the new command line flag proposed in bug 383185:

 -bidi "on=[y/n];textDir=[ltr/rtl/auto]"
Comment 1 Markus Keller CLA 2013-03-08 08:34:26 EST 
Created attachment 228136 [details]
Fix

Can we fix this for M6? I can release the fix unless there's opposition.
Comment 2 Silenio Quarti CLA 2013-03-08 11:01:44 EST 
My only concern is whether this will case performance degradation on Windows. Do you have any numbers?

The command line switch will only help eclipse plugins. I am not a big fun of system properties, but we could add this line to the beginning of isBidi():

if (!"true".equals(System.getProperty("org.eclipse.swt.checkBidiLocale"))) return true;
Comment 3 Markus Keller CLA 2013-03-08 12:04:32 EST 
It will cause more BidiSegmentListener callbacks, but it can't be a relevant performance degradation, since we already accept this unconditionally on Mac and GTK, and on Windows platforms where a bidi keyboard language is installed.

In the Eclipse SDK, the only relevant BidiSegmentListener is in the JavaSourceViewer. This is called every time a line is rendered in a Java editor. When I scroll 1000 lines of StyledText (takes about 30s here), both YourKit and System.currentTimeMillis() have a hard time measuring any time spent in StyledText#getBidiSegments(..). Often, it's 0ms, sometimes 1ms. Accumulated over the 30s, it's around 30ms.

I think we should just remove this and we don't need a system property.
Comment 4 Silenio Quarti CLA 2013-03-08 13:21:59 EST 
OK, +1 to your patch.