Community
Participate
Working Groups
Need to investigate this to find out why.
Heba (did I get your name right?), do you have any ideas?
Hello Steve, yes you got my name correctly:) The problem in style text with RTL, is that : 1- Style text is not a native widget so it doesn't inherit the RTL support from the OS. 2- Mirroring is not handled style text drawing methods. So the solution is to implement the mirroring support in it. I noticed that only the cursor has some mirrored behavior, it goes from right to left however all the text positioning and selection is not handled in the case of RTL. This needs to have mirroring added during drawing time. Some how should be similar to the right alignment behavior. PS: From a quick investigation I was not able to find where the cursor mirroring is done !!! Please let me know if I can be of more help
I will add Rasha and Mati with us on the discussion
Custom widgets use the GC that is provided when the widget is painted and this GC is mirrored. Also I believe that when a GC is created on a widget outside of paint, this GC is also mirrored. This means that most custom widgets (like CTabFolder) work 99% of the time without doing anything. This also means that StyledText should work mostly without doing anything as well but it does not. StyledText is doing something different to draw it's contents. Whatever it is doing that is causing the failure needs to be fully understood. Other similiar application code outside of SWT could fail in the same way.
Created attachment 4040 [details] StyledText.java from RC1 build with proposed changes,which are marked with "rtl" tag.
Created attachment 4041 [details] StyledTextBidi.java from RC1 build with proposed changes
Created attachment 4042 [details] BidiUtil.java from RC1 build with proposed changes
Created attachment 4044 [details] OS.java from RC1 build with proposed changes
Created attachment 4047 [details] swt.c from RC1 buid ( GetLayout() added )
Created attachment 4067 [details] Caret.java from RC1 build with proposed changes
I've code reviewed all the changes. That's what I think: OS.java and swt.c are okay BidiUtil.java: in drawGlyphs we have the line "renderDx[length - 1]--;" after swap the dx array, but in getRenderInfo we don't have the same line after swapping the dx array, is that important ? Caret.java the methods killFocus/setFocus can not be public. StyledTextBidi.java: It can not have a reference to win32 internal. We may need to add a new API in GC to know if it's mirrored, something like GC.isMirrored() or GC.getMirrored(). I don't like the field isRTL, it should be renamed to isMirroredGC or similar, for instance, in getCaretOffsetAndDirectionAtX we have: if (rightToLeft) if (visualLeft) if (isRTL) direction = previous; else direction = next; else if (isRTL) direction = next; else direction = previous; else if (visualLeft) if (isRTL) direction = next; else direction = previous; else if (isRTL) direction = previous; else direction = next; I mean, we have rightToLeft and RTL been test togheter, I think this is confusing. This nest if's and else's in getCaretOffsetAndDirectionAtX and getTextPosition are bad. StyledText.java Again, it can't have reference to win32 internal, we may need to add a GC.setMirrored(boolean mirrored) API. In the setBounds, we must find a solution for the Caret problem that do not uses internal APIs, I've been able to fix that by saving the image and the bounds of the Caret, disposing the caret, call super.setBounds (), and at end create a new Caret setting the image and bounds I've saved. Comments ?
Okay, we have a better fix for the Caret problem: 1) remove setBounds() of StyledText 2) remove the changes in killFocus and setFocus in Caret 3) in Caret add the method: void updateCaret () { moved = true; if (isVisible && hasFocus ()) move (); } 4) in Canvas add the method: LRESULT WM_SIZE (int wParam, int lParam) { LRESULT result = super.WM_SIZE (wParam, lParam); if (caret != null) caret.updateCaret (); return result; } Still, in order to release these fixes we need to decide about the APIs setMirrored()/getMirrored() in GC.
BidiUtil.java : getRenderInfo() determines char positions correctly, and renderDx can be used as is. Problem appears when ExtTextOut() is used with mirrored DC. It increases width of typed text by one pixel. Because segments of text in this case are displayed from right to left, each next segment damages previous. StyledTextBidi.java: other name of field - OK. Caret problem: as far as I understand, when canvas is mirrored, caret should be hidden before canvas is resized. In opposite case we can receive row of StyledText with many "ghosts" of caret (quantity of them depend on speed of resizing). WM_SIZE is sent after size of canvas was changed, therefore using of updateCaret() can't solve this problem. Therefore I suggest to transfer method getBounds() from StyledText to Canvas. There it can look as the following: public void setBounds(int x, int y, int width, int height) { boolean caretVisible = false; boolean isRTL = (style & SWT.RIGHT_TO_LEFT) != 0; if (isRTL && caret != null) { caretVisible = caret.getVisible(); caret.setVisible(false); } super.setBounds(x,y,width,height); if (caretVisisble) caret.updateCaret(); } ...where method updateCaret() of Caret can look as the following: void updateCaret() { moved = true; setVisible(true); }
I see, my code works on WinXP but it fails on W2k.
Okay, this code works for me on W2k and WinXP: void setBounds(int x, int y, int width, int height, int flags) { boolean isFocus = (style & SWT.RIGHT_TO_LEFT) != 0 && caret != null && caret.isFocusCaret (); if (isFocus) caret.killFocus (); super.setBounds (x, y, width, height, flags); if (isFocus) caret.setFocus (); }
In OS.java, you guys had: public static final int LAYOUT_BITMAPORIENTATIONPRESERVED = 0x8; but it's not been used, right ? it's there just in case then ?
Yes, currently it isn't used.
Fix for Caret was released. new API for mirroring a GC has been added. we are still working on: BidiUtil.java, StyledText.java, and StyledTexBidi.java
Created attachment 4776 [details] StyledText.java from build20030502, changes marked with "rtf" flag
Created attachment 4777 [details] StyledTextBidi.java from build20030502, changes marked with "rtf" flag
Created attachment 4778 [details] BidiUtil.java from build20030502, changes marked with "rtf" flag
Knut, we already did all the changes need in the SWT side to support Bidi in a SWT.LEFT_TO_RIGHT oriented StyledText. I've changes the original files created by Semion to use the new style bit. NOTE: This is a P2. SN says, "You need to look at this!".
I'll have a look. Thanks for doing the work!
I had trouble importing these files. I got bogus text at the end of the *.java files. How did you attach these?
Created attachment 4791 [details] StyledTex, StyledTextBidi, BidiUtil
Entered bug 37293 for GC.drawImage not working properly on a mirrored canvas. The attached StyledText currently works around this in performPaint. The +1/-1 arithmetic should not be necessary. Also, there is resize cheese to the left of the rendered lines. The area is not cleared with the proper background when resizing the widget wider. You can see this when using a non-white window background. To fix this there needs to be another +1 patch in StyledTextRenderer.drawLine. This is not neccessary if bug 37293 is fixed. The cheese is caused by the workarounds in performPaint. Entered bug 37294 for the fact that the vertical scroll bar is not mirrored in the RIGHT_TO_LEFT canvas, the horizontal scroll bar is mirrored.
We have reviewed the code and have the following questions about it: 1. Why is the following code in the StyledText.redraw method? super.redraw(x-(isRTL? BIDI_CARET_WIDTH:0), y, width+(isRTL? BIDI_CARET_WIDTH:0), height, all) We do not think this is necessary and have removed the code. 2. We removed the magic plus/minus 1 arithmetic in StyledText.setBidiCaretLocation and StyledText.performPaint 3. In BidiUtil.drawGlyphs we see that decrementing the last dx value solves the overlapping segment problem that you mention above. However, we're not sure this is really necessary. Until [Bug 37293] is fixed, it's not worth investigating this further. For now, we have removed the code. 4. Note that for some fonts, the caret overlaps characters for some fonts. In your original code, you can see the overlap (Microsoft Sans Serif). When we take out the magic dx decrementing, you can see the caret overlap for Tahoma. Both Notepad and WordPad exhibit the same behavior. 5. Should we be supporting Right-To-Left style when in non-bidi mode? If so, the current rendering of text is kind of strange. For example, if we do not have any bidi languages installed (i.e., no hebrew/arabic keyboard specified) and we enter the following text: This is a test. The text will be rendered as: .This is a test This seems incorrect. We think we can come up with a test case using just a canvas and drawString to show the problem. We'll open a bug report. In the case when we're in non-bidi mode with RtoL orientation, cursor positioning does not behave correctly. This will also have to be fixed in order to correctly support RtoL orientation in non-bidi mode. OR DO WE MEAN TO HAVE RTOL ORIENTATION TO SOLELY APPLY TO BIDI? Felipe please check with Steve. 6. Why wasn't CTRL-SHIFT support put in to support dynamic orientation switching? We think this should be supported. It would also require us to dynamically change the orientation of the canvas (new SWT API).
For 5. drawString reorders text when RIGHT_TO_LEFT is specified and a bidi system locale is enabled. This is generally expected when rendering text. However, for StyledText this means that we will display reordered text even if the widget is not in bidi mode. We use the installed keyboard languages in addition to the locales to determine if we should be in bidi mode. The result is that there will not be a bidi caret in this scenario. This may be an unlikely scenario anyway, running with a hebrew locale but without hebrew keyboard support.
About magic +1/-1 arithmetic: I saw problem, but I don't think, that it is a problem of drawImage only. Can you, for example, do mouse click in the position (0,0) of any rtl-oriented SWT widget? I can't, the first available position has (1,0) coordinates... About the code in the StyledText.redraw method: I worked with RTL, but problem here isn't RTL specific. Some characters of some fonts require actually more place, then GetCharacterPlacement() informs about them. Let us use, for example, font Tahoma, size 36, text "sj" in the ltr-oriented StyledText and perform selection using Shift+RightArrow. We can see, that glyph "j" "steal" one pixel from rectangle, where "s" is displayed. Let us select "s". Method redraw(), which is called here, asks to invalidate rectangle,used to display this character. After that drawStyledLine() displays two segments of text with different backgrounds, and when ExtTextOut() write character "j", background of "j" damages background of "s". Now we add to selection symbol "j". Method redraw() asks to invalidate corresponding rectangle, using information from getRenderInfo(), but X-position of this rectangle is greater, then "j" actually used. Therefore line with width of one pixel and default background remains between two selected characters... I think, we can avoid this problem, if width of invalidated area should be enlarged. It can include, for example, width of previous,current and next characters. If we don't have previous or next character, BIDI_CARET_WIDTH can be used instead of their width.
I added a comment about the 1,0 origin to bug 37293. The measurement problem that you work around in redraw is actually documented in bug 4776. It may be worth investigating if transparent rendering fixes the cheese as suggested in that bug. The problem gets worse as the font size gets bigger. Tahoma and Microsoft Sans Serif up to size 13 look fine.
We verified that when bug 37293 gets fixed no workaround in redraw (int,int,int,int,boolean) will be necessary. The bug 4776 has the same failure behavior as the case that the redraw change was trying to handle, but it is a different problem - specific to certain arabic fonts. Bottomline - we see no reason for any of the magic values.
Bug 37293 only addressed the drawImage problem (off by one with cheese on the right edge). Opened bug 37427 for the GC/Canvas origin off by one problem. When that bug is fixed we no longer need a workaround in redraw. Note that there is also cheese when scrolling horizontally. This is likely caused by the same bug.
Lynne, with regarding to item 5, comment #27: I've tested the scenario you described on my machine (winXP which has arabic keyboard and arabic language installed), silenio's machine (winXP, no arabic at all), and the arabic machine in SWT Lab (win98) and I have the same problem in them all, I typed Felipe. and the StyledText shows .Felipe Actually, I also test with notepad, running on the arabic machine, and it also shows .Felipe when I type Felipe. So, maybe it's the right behavior ? Semion, could you add comments to this problem ?
If you type "Felipe." and receive ".Felipe" with RTL DC, it is correct behaviour, because text is reordered in rtl mode. As far as I understand, question here is: "Should text be reordered in rtl mode, if any bidi locale isn't available?". From my oppinion, it is more correct to ask, if layout of DC can be changed to rtl, when any bidi locale isn't available. If answer to this question is "no", we should start with this point. If answer to this question for some reason is "yes", we should use the corresponding mode of text reordering. Mati, what do you think about this?
Here is my position on the issue in comment #34. I don't think that honoring the RTL flag should be dependent on the presence of a Bidi locale in the system, especially if the meaning of "Bidi locale" means the presence of a keyboard mapping for a Bidi language, as it seems to be in the case of Windows. If I want to send a Hebrew document to a friend in Italy, why should he need to install a Bidi locale to see it? We could argue that there is no point to display in RTL style if there is no Bidi data in the text or Bidi font to present the text. Both conditions are a pain to check (specially the font issue), and satisfying them would remove the ability to check Bidi presentation with simulated Bidi data, which is handy at development time. So it seems to me that if the RTL style is set, it means that the user wants RTL presentation, even if the data is "Felipe.", and it should come as no surprise if the presentation is ".Felipe", this is just honoring the request, which is what the system should do, and not question the validity of the request itself.
StyledText BIDI support fails on W2K. I have a StyledText component created with the following flags: SWT.BORDER | SWT.MULTI | SWT.V_SCROLL | SWT.V_SCROLL |SWT.WRAP |SWT.RIGHT_TO_LEFT My locale is set to Arabic (Saudi) and my keyboard is set to Arabic. Vertical scroll bar is displayed at the left, as it should, but the text is aligned at the left (wrong). Caret moves as if the text was right aligned and does not have any relationship with the text actually displayed.
Rodolfo, all of the changes for RtoL alignment support in the StyledText widget have not been released into the development stream.
Lynne, I'm running Integration build I20030507 now. Can you tell me in what build will the changes appear?. I need this fix urgently. Thanks in advance.
We are actively working on the StyledText changes to support this and these changes should be released soon. But there are some open issues below us (Bug 37427).
One more problem to work out in our right orientation code: If you enter numbers followed by hebrew text (i.e., the numbers are local numbers, not latin numbers) there is no way to cursor to the last number that was entered. E.g., werbeh123 the cursor can not be placed to the right of the 3. This should be possible when pressing cursor next. This causes problems in the auto scroll code. It is related to the problem noted in bug 37449.
I am not sure, that I fully understand your example. You typed some digits, after that - hebrew text ("werbeh123" is result of rendering, not logical contents of buffer, which is "123hebrew"), and now you try to place caret from the right of the last digit(in your example it is "3"), using of right arrow keys ("cursor previous"). Right? If I understand this correct, I don't see, how it is related to the problem noted in bug 37449. Let us suppose, that cursor is placed between "e" and "h", and you press right arrow. Now cursor should be placed between "h" and "3" - or from right of "h" (before "h"), or from right from "3" (after "3"). Each of these positions is correct, but we choose first of them, because cursor was moved from side of "h". In opposite case, when cursor is placed between "2" and "3" and you press left arrow, cursor should be placed in the same point of the logical buffer, but we choose second visual position (from the right of "3") - in accordance with the same logic.
We introduced the bug I mention in comment #41 when we refactored the right orientation code. It's fixed now. It was in StyledTextBidi.getTextPosition.
After much discussion, we've decided StyledText needs to work around Bug 37427/Bug 4776. We will put back the magic + 1 in StyledText.redraw (int,int,int,int, boolean). We investigated using a transparent background during ExtTextOut call, but doing so introduced a bug during discontiguous selection. We did not investigate the option any further since the change to the redraw method solves the problem in a much simpler way with less platform calls.
Mati, in response to comment #35: Even if we only allow right orientation when our bidi requirements are met (bidi locale and bidi keyboard), you would still be able to see bidi text in your example. It would just not be right oriented. That said, we have no problem always supporting right orientation. We probably underestimate the importance of right-to-left writing order when a document is in hebrew/arabic. The remaining open question is supporting dynamic switching of the writing order (via API and keyboard shortcut). See comment #27 item 6. We should probably put this in.
In response to comment #44 : Please don't underestimate the importance of right to left writing mode. I wrote an XLIFF translation editor using SWT. It works OK with _ANY_ language except BiDi ones. People translating to/from BiDi languages must be able to type text without problems. If the application sets SWT.RIGHT_TO_LEFT style, then all text should be treated as BiDi. It is the responsibility of the programmer to check wheter the text contains BiDi text or not when creating the component. IMHO, using CTRL+SHIFT+something (comment #27 - item 6) to switch between BiDi or normal order is a bad idea. Bidi and non-bidi modes should be selected at component creation time using a flag, like SWT.RIGHT_TO_LEFT
About comment 27, item 6. Just to let you know, CTRL+SHIT is used by chinese input method in Linux. Therefore, StyledText would never see CTRL+SHIT or it would steal it from the input method (depending which one runs first: filterEvent() or translateAccelerator()). Also, there is a Bug#29779 about changing widget orientation in runtime. StyledText should not hardcode a keybinding for it, once a API to change the widget orientation in runtime is available the application can set any keybinding they want to do a dynamic orientation switching.
Yes Felipe, the ability to change orientation at runtime will be sufficient. We weren't aware of [Bug 29779].
Why not use a default key combination to toggle the orientation? Java and AIX use Alt+Enter so, since Ctrl+Shift is not possible, this could be a better alternative.
Changes released in build > 20030519.
Released to HEAD, marking milestone.
*** Bug 37895 has been marked as a duplicate of this bug. ***
Note that the fix is not in the 20030520 integration build. It is in the 20030524 nightly build and will be in this week's integration build.
Fixed 2.1.2
Semion is this OK in the current BIDI support?
