Re: winemac.drv: Improve positioning of IME client window based on caret
On Feb 19, 2014, at 3:08 AM, Aric Stewart wrote:
diff --git a/dlls/winemac.drv/ime.c b/dlls/winemac.drv/ime.c index 2c74f6e..066c77a 100644 --- a/dlls/winemac.drv/ime.c +++ b/dlls/winemac.drv/ime.c @@ -1585,14 +1585,20 @@ BOOL query_ime_char_rect(macdrv_query* query) if (focus && (focus == hwnd || IsChild(hwnd, focus)) && GetClientRect(focus, &charpos.rcDocument)) { + HDC hdc = GetDC(focus); + TEXTMETRICA tm; + if (!GetCaretPos((POINT*)&charpos.rcDocument)) charpos.rcDocument.left = charpos.rcDocument.top = 0;
+ GetTextMetricsA(hdc, &tm); charpos.rcDocument.right = charpos.rcDocument.left + 1; + charpos.rcDocument.bottom = charpos.rcDocument.top + tm.tmHeight; MapWindowPoints(focus, 0, (POINT*)&charpos.rcDocument, 2);
*rect = cgrect_from_rect(charpos.rcDocument); ret = TRUE; + ReleaseDC(focus, hdc); } }
I don't think this does what you want. GetDC() typically returns a DC whose parameters were reset. It doesn't know what font has previously been used in the window. So, you're basically getting the metrics for the System font. I suppose that may be better than nothing. Also, you don't check if GetTextMetricsA() succeeded. Perhaps using GetGUIThreadInfo() and the rcCaret field of the GUITHREADINFO structure would be better than using GetCaretPos(). In that case, you'd map the coordinates from hwndCaret (which may be different from "focus") to the screen. Be sure to read the remarks in the MSDN docs for the GetGUIThreadInfo() function about how to use rcCaret, although it's not clear to me if Wine implements this faithfully. -Ken
On 2014/02/20 0:32, Ken Thomases wrote:
On Feb 19, 2014, at 3:08 AM, Aric Stewart wrote:
diff --git a/dlls/winemac.drv/ime.c b/dlls/winemac.drv/ime.c index 2c74f6e..066c77a 100644 --- a/dlls/winemac.drv/ime.c +++ b/dlls/winemac.drv/ime.c @@ -1585,14 +1585,20 @@ BOOL query_ime_char_rect(macdrv_query* query) if (focus && (focus == hwnd || IsChild(hwnd, focus)) && GetClientRect(focus, &charpos.rcDocument)) { + HDC hdc = GetDC(focus); + TEXTMETRICA tm; + if (!GetCaretPos((POINT*)&charpos.rcDocument)) charpos.rcDocument.left = charpos.rcDocument.top = 0;
+ GetTextMetricsA(hdc, &tm); charpos.rcDocument.right = charpos.rcDocument.left + 1; + charpos.rcDocument.bottom = charpos.rcDocument.top + tm.tmHeight; MapWindowPoints(focus, 0, (POINT*)&charpos.rcDocument, 2);
*rect = cgrect_from_rect(charpos.rcDocument); ret = TRUE; + ReleaseDC(focus, hdc); } }
I don't think this does what you want. GetDC() typically returns a DC whose parameters were reset. It doesn't know what font has previously been used in the window. So, you're basically getting the metrics for the System font. I suppose that may be better than nothing.
I never knew that about GetDC, I did not see anything about reset parameters in MSDN. It was working for me pretty accurately in my testing even with different fonts. Is there a way to get the font metrics from a window handle? I am going to need that for when I am implementing the Candidate Window stuff also.
Also, you don't check if GetTextMetricsA() succeeded.
True.
Perhaps using GetGUIThreadInfo() and the rcCaret field of the GUITHREADINFO structure would be better than using GetCaretPos(). In that case, you'd map the coordinates from hwndCaret (which may be different from "focus") to the screen. Be sure to read the remarks in the MSDN docs for the GetGUIThreadInfo() function about how to use rcCaret, although it's not clear to me if Wine implements this faithfully.
I will look into this, I have a number of programs where GetCaretPos is not returning correct information anyway so an alternative would be useful. -aric
On Feb 19, 2014, at 3:42 PM, Aric Stewart wrote:
On 2014/02/20 0:32, Ken Thomases wrote:
I don't think this does what you want. GetDC() typically returns a DC whose parameters were reset. It doesn't know what font has previously been used in the window. So, you're basically getting the metrics for the System font. I suppose that may be better than nothing.
I never knew that about GetDC, I did not see anything about reset parameters in MSDN.
It's the first paragraph of the Remarks. <http://msdn.microsoft.com/en-us/library/windows/desktop/dd144871%28v=vs.85%29.aspx> Don't be confused by the last sentence with its mention of GetTextFace(). That's not a solution to the problem described in the rest of the paragraph; it's a non sequitur.
It was working for me pretty accurately in my testing even with different fonts. Is there a way to get the font metrics from a window handle?
No. This isn't a property of the window in the common case. It's only a property of the DC, which may exist only temporarily while drawing. And one may select various fonts into the DC at different times to draw different parts of the window. -Ken
On 2014/02/20 6:57, Ken Thomases wrote:
On Feb 19, 2014, at 3:42 PM, Aric Stewart wrote:
On 2014/02/20 0:32, Ken Thomases wrote:
I don't think this does what you want. GetDC() typically returns a DC whose parameters were reset. It doesn't know what font has previously been used in the window. So, you're basically getting the metrics for the System font. I suppose that may be better than nothing.
I never knew that about GetDC, I did not see anything about reset parameters in MSDN.
It's the first paragraph of the Remarks. <http://msdn.microsoft.com/en-us/library/windows/desktop/dd144871%28v=vs.85%29.aspx> Don't be confused by the last sentence with its mention of GetTextFace(). That's not a solution to the problem described in the rest of the paragraph; it's a non sequitur.
It was working for me pretty accurately in my testing even with different fonts. Is there a way to get the font metrics from a window handle?
No. This isn't a property of the window in the common case. It's only a property of the DC, which may exist only temporarily while drawing. And one may select various fonts into the DC at different times to draw different parts of the window.
humm, so how should i determine line height? -aric
On Feb 19, 2014, at 4:19 PM, Aric Stewart wrote:
humm, so how should i determine line height?
I don't think you can. It's not a well-formed concept. Windows don't have any inherent line height property (directly or indirectly). Have you tried the GetGUIThreadInfo() approach and rcCaret? You're trying to fix the case when using GetCaretPos() but maybe we shouldn't be using that. If you're thinking forward to IMC_SETCANDIDATEPOS, then a horrible kludge might be to return a rect whose bottom left is at the specified point and which extends up and to the right to the screen boundaries. The idea is to try to make the input method position its candidate window so its top-left is at the specified point. It will likely align its left side with the left side of the character rect we return (for left-to-right languages, at least) and, by taking up the area of the screen above and to the right of that point, you force the candidate window to be below the rect. -Ken
On 2014/02/20 7:46, Ken Thomases wrote:
On Feb 19, 2014, at 4:19 PM, Aric Stewart wrote:
humm, so how should i determine line height?
I don't think you can. It's not a well-formed concept. Windows don't have any inherent line height property (directly or indirectly).
Have you tried the GetGUIThreadInfo() approach and rcCaret? You're trying to fix the case when using GetCaretPos() but maybe we shouldn't be using that.
If you're thinking forward to IMC_SETCANDIDATEPOS, then a horrible kludge might be to return a rect whose bottom left is at the specified point and which extends up and to the right to the screen boundaries. The idea is to try to make the input method position its candidate window so its top-left is at the specified point. It will likely align its left side with the left side of the character rect we return (for left-to-right languages, at least) and, by taking up the area of the screen above and to the right of that point, you force the candidate window to be below the rect.
I am thinking forward to the IMC_SETCANDIDATEPOS implementation, but on the way found that our caret pos version was seriously bad and was working on that first. The issue with simply kludging the rect like that is that it appears that the caret position that we are given is in the middle of the line (not the bottom where it would be useful) so we end up clipping the bottom half of the composition string with the selection dialog. Again lending to the need to be able to at least guess at a line height to shift the box down by. -aric
On 2014/02/20 7:46, Ken Thomases wrote:
On Feb 19, 2014, at 4:19 PM, Aric Stewart wrote:
humm, so how should i determine line height?
I don't think you can. It's not a well-formed concept. Windows don't have any inherent line height property (directly or indirectly).
Have you tried the GetGUIThreadInfo() approach and rcCaret? You're trying to fix the case when using GetCaretPos() but maybe we shouldn't be using that.
If you're thinking forward to IMC_SETCANDIDATEPOS, then a horrible kludge might be to return a rect whose bottom left is at the specified point and which extends up and to the right to the screen boundaries. The idea is to try to make the input method position its candidate window so its top-left is at the specified point. It will likely align its left side with the left side of the character rect we return (for left-to-right languages, at least) and, by taking up the area of the screen above and to the right of that point, you force the candidate window to be below the rect.
-Ken
GetGUIThreadInfo is totally the correct way to go. It is giving me correct caret rects for both notepad and the Japanese applications I am trying to fix. I can worry less about the not about caret insertion points because i do not care about the width, just the height and position. so a width of 1 is fine. I will resubmit the patch with this logic. Thanks! -aric
On Feb 19, 2014, at 9:00 PM, Aric Stewart wrote:
GetGUIThreadInfo is totally the correct way to go. It is giving me correct caret rects for both notepad and the Japanese applications I am trying to fix. I can worry less about the not about caret insertion points because i do not care about the width, just the height and position. so a width of 1 is fine.
Well the note wasn't about the caret width, it was about the proper computation of the left edge. According to MSDN, rcCaret.left is not correct as given. As I said, though, I'm not sure Wine is doing the right thing with that, anyway. The problem may only show up with right-to-left or Thai input. *shrug* Anyway, glad it's giving useful results. -Ken
On 2014/02/20 12:12, Ken Thomases wrote:
On Feb 19, 2014, at 9:00 PM, Aric Stewart wrote:
GetGUIThreadInfo is totally the correct way to go. It is giving me correct caret rects for both notepad and the Japanese applications I am trying to fix. I can worry less about the not about caret insertion points because i do not care about the width, just the height and position. so a width of 1 is fine.
Well the note wasn't about the caret width, it was about the proper computation of the left edge. According to MSDN, rcCaret.left is not correct as given. As I said, though, I'm not sure Wine is doing the right thing with that, anyway. The problem may only show up with right-to-left or Thai input. *shrug*
True, but I am not really worried about the insertion point to that degree of accuracy. Just the location of the cursor itself and the height. Both of which appear to be correct. Thanks for the tip on this. -aric
participants (2)
-
Aric Stewart -
Ken Thomases