Re: [3/3] gdiplus: Make GdipMeasureCharacterRanges/GdipMeasureString/GdipDrawString add extra space around the text.
These APIs use different logic for adding extra space, so at the moment I don't see how gdip_format_string() can be used to do this in one place, but if somebody has an idea how to do that - patches are welcome.
That is highly unlikely, particularly given that MeasureCharacterRanges is the accepted practice for highlighting parts of a string.
You should make sure you do your tests so the size in pixels is the em size of the font, or at least at a consistent size between them, as native may cut into the padding if hinting makes the characters proportionally larger (this is the point of having the padding, but for the record I think it is behavior that we shouldn't try to duplicate).
Do you have a visual test program for this? I'd like to see something that draws a string overlayed with the measurements, to confirm that the functions are not consistent on Windows (or, if they are, confirm that they are consistent with your patch). If not, I'll write one and send you the source code.
+ /* FIXME: GenericTypographic format prevents extra margins */
Since no margins is the current behavior, won't that cause a regression? It should be fairly easy to include that exception.
Then again, I think I've heard reports that GenericTypographic doesn't work.
Vincent Povirk madewokherd@gmail.com wrote:
These APIs use different logic for adding extra space, so at the moment I don't see how gdip_format_string() can be used to do this in one place, but if somebody has an idea how to do that - patches are welcome.
That is highly unlikely, particularly given that MeasureCharacterRanges is the accepted practice for highlighting parts of a string.
For instance accoding to the tests MeasureCharacterRanges adds extra space only to X, while GdipMeasureString and GdipDrawString also add extra space to Height and Width of the bounding rectangle.
You should make sure you do your tests so the size in pixels is the em size of the font, or at least at a consistent size between them, as native may cut into the padding if hinting makes the characters proportionally larger (this is the point of having the padding, but for the record I think it is behavior that we shouldn't try to duplicate).
That's an implementation detail, and my patch has nothing to do with that.
Do you have a visual test program for this? I'd like to see something that draws a string overlayed with the measurements, to confirm that the functions are not consistent on Windows (or, if they are, confirm that they are consistent with your patch). If not, I'll write one and send you the source code.
I have an application which makes a havy use of all of this.
- /* FIXME: GenericTypographic format prevents extra margins */
Since no margins is the current behavior, won't that cause a regression? It should be fairly easy to include that exception.
Then again, I think I've heard reports that GenericTypographic doesn't work.
GenericTypographic does work in my tests, but since it's currently completely ignored anyway I decided to leave it alone. Once these patches are in I'll add support for this case later.
Dmitry Timoshkov dmitry@baikal.ru wrote:
Vincent Povirk madewokherd@gmail.com wrote:
These APIs use different logic for adding extra space, so at the moment I don't see how gdip_format_string() can be used to do this in one place, but if somebody has an idea how to do that - patches are welcome.
That is highly unlikely, particularly given that MeasureCharacterRanges is the accepted practice for highlighting parts of a string.
For instance accoding to the tests MeasureCharacterRanges adds extra space only to X, while GdipMeasureString and GdipDrawString also add extra space to Height and Width of the bounding rectangle.
GdipMeasureString and GdipDrawString add extra space to Y too.
For instance accoding to the tests MeasureCharacterRanges adds extra space only to X, while GdipMeasureString and GdipDrawString also add extra space to Height and Width of the bounding rectangle.
That doesn't sound inconsistent to me, it's just that those functions are measuring different things. CharacterRanges tells you where the individual characters are (and is resolution-dependent as I understand it), while MeasureString/DrawString only give information about full lines and are resolution-independent (they may have to cut into the padding depending on the hinting at that resolution, but users of these functions can ignore that and assume the entire line will scale to any resolution).
Probably the callback just needs another argument specifying where the first character is. (For tabs we may need a position for subsequent characters as well, or multiple callbacks for a single line.)
I don't think we should make MeasureString/DrawString resolution-independent because that also makes native's text incredibly ugly, and would greatly increase the complexity of gdip_format_string.
Vincent Povirk madewokherd@gmail.com wrote:
For instance accoding to the tests MeasureCharacterRanges adds extra space only to X, while GdipMeasureString and GdipDrawString also add extra space to Height and Width of the bounding rectangle.
That doesn't sound inconsistent to me, it's just that those functions are measuring different things. CharacterRanges tells you where the individual characters are (and is resolution-dependent as I understand it), while MeasureString/DrawString only give information about full lines and are resolution-independent (they may have to cut into the padding depending on the hinting at that resolution, but users of these functions can ignore that and assume the entire line will scale to any resolution).
Probably the callback just needs another argument specifying where the first character is. (For tabs we may need a position for subsequent characters as well, or multiple callbacks for a single line.)
That's the whole reason of my comment: "at the moment I don't see how gdip_format_string() can be used to do this in one place, but if somebody has an idea how to do that - patches are welcome."
Right now I just want to implement correct behaviour to make the tests pass, nothing more. Any optimizations, simplifications, or even redesign could be done later.
I don't think we should make MeasureString/DrawString resolution-independent because that also makes native's text incredibly ugly, and would greatly increase the complexity of gdip_format_string.
That sounds like a pure guess, not related to my patches.
I don't think we should make MeasureString/DrawString resolution-independent because that also makes native's text incredibly ugly, and would greatly increase the complexity of gdip_format_string.
That sounds like a pure guess, not related to my patches.
Nope, I read about it here: http://support.microsoft.com/kb/307208
And since it's the rationale for the padding and the reason we can't duplicate native's measurements exactly, I think it's relevant to the discussion.
Vincent Povirk madewokherd@gmail.com wrote:
I don't think we should make MeasureString/DrawString resolution-independent because that also makes native's text incredibly ugly, and would greatly increase the complexity of gdip_format_string.
That sounds like a pure guess, not related to my patches.
Nope, I read about it here: http://support.microsoft.com/kb/307208
And since it's the rationale for the padding and the reason we can't duplicate native's measurements exactly, I think it's relevant to the discussion.
IMHO the whole "resolution-independent" term in that article is a pure speculation to justify inventing and adding new shiny technology to Windows. GDI32 does all its rendering in "resolution-independent" manner just fine for a very long time already.
IMHO the whole "resolution-independent" term in that article is a pure speculation to justify inventing and adding new shiny technology to Windows.
s/n/tt
Vincent Povirk madewokherd@gmail.com wrote:
IMHO the whole "resolution-independent" term in that article is a pure speculation to justify inventing and adding new shiny technology to Windows.
s/n/tt
If there are only cosmetic objections, I'd like to see this patch accepted, and work of further improvements after that.
If there are only cosmetic objections, I'd like to see this patch accepted, and work of further improvements after that.
I asked Alexandre to wait until I had more time to understand/test this patch.
It seems you've introduced a problem with scaling (see attached test program and screenshot). I think that MeasureString is working correctly in this case, and DrawString/MeasureCharacterRanges are wrong.
This sort of problem would be less likely if the logic were in gdip_format_string.
Vincent Povirk madewokherd@gmail.com wrote:
If there are only cosmetic objections, I'd like to see this patch accepted, and work of further improvements after that.
I asked Alexandre to wait until I had more time to understand/test this patch.
It seems you've introduced a problem with scaling (see attached test program and screenshot). I think that MeasureString is working correctly in this case, and DrawString/MeasureCharacterRanges are wrong.
Thanks for the test program, it's pretty illustrative. I sent another version of the patch that should take scaling and world transform into account.
This sort of problem would be less likely if the logic were in gdip_format_string.
I still think that it's more important to make these APIs behave correctly first, and only then try to optimize/merge/redesign the logic behind them.
-
Dmitry Timoshkov -
Vincent Povirk