What’s wrong with this code, part 22 – Drawing Text…
Recently I’ve been working on something that I’ve never done before in my almost 24 years at Microsoft.
For the past 23ish years, I’ve been a plumber – all the work I’ve done has been under the covers. But for the next version of Windows, I decided to stretch my boundaries a bit and try some UI programming. I’ve just spent the past few days working on a cool change to the volume control (it’s not important what it is, and most people will never know about the change, but those that do will probably agree with me :)).
As part of the change, I needed to measure the dimensions of a text string. This is a dummy version of some code I wrote, I simply called DrawText with the DT_CALCRECT into a memory DC that I created.
BOOL InitInstance(HINSTANCE hInstance, int nCmdShow)
{
HWND hWnd;
hInst = hInstance; // Store instance handle in our global variable
hWnd = CreateWindow(szWindowClass, szTitle, WS_OVERLAPPEDWINDOW,
CW_USEDEFAULT, 0, CW_USEDEFAULT, 0, NULL, NULL, hInstance, NULL);
if (!hWnd)
{
return FALSE;
}
<BEGIN LARRYS CODE>
HDC hdc = CreateCompatibleDC(NULL);
RECT rcText = {0, 0, 88, 34};
DrawText(hdc, L"My Text String", -1, &rcText, DT_CENTER | DT_END_ELLIPSIS | DT_EDITCONTROL | DT_WORDBREAK | DT_NOPREFIX | DT_CALCRECT);
CAtlString string;
string.Format(L"Text String occupies: %d x %d pixels", rcText.right - rcText.left, rcText.bottom - rcText.top);
MessageBox(hWnd, string, L"String Size", 0);
<END LARRYS CODE>
ShowWindow(hWnd, nCmdShow);
UpdateWindow(hWnd);
return TRUE;
}
This is just code I took by using Visual Studio to create a Windows Win32 project and inserting the code between “BEGIN LARRYS CODE” and “END LARRYS CODE”. The meat of the code is just 3 lines of code.
Even though there’s almost no code here, it still has a bug in it that was quite subtle and took me several hours to find.
Comments
Anonymous
August 01, 2008
CreateCompatibleDC requires that you call DeleteDC when you are done with the handle, so there's a GDI handle leak.Anonymous
August 01, 2008
Aaron, Doh! Stupid bug, but not the one I was looking for.Anonymous
August 01, 2008
Why would you use DT_END_ELLIPSIS if you're trying to measure the rectangle you need to fit the string in?Anonymous
August 01, 2008
Will, that's a good question that's tied up with the actual use of the code. It's not germane to the bug however. I could probably get rid of the DT_ENDELLIPSIS in my code actually.Anonymous
August 01, 2008
You wrote that you needed the dimensions of the text but you also init'd the rect. Not clear if you meant the centered, word broken dimensions or if those flags are the problem (DT_WORDBREAK will look at the rect dimensions).Anonymous
August 01, 2008
Must your first application window be visible before calling CreateCompatibleDC with NULL?Anonymous
August 01, 2008
Don't you need to call CreateCompatibleBitmap before using the DC? Or is it not required because you aren't really drawing to it?Anonymous
August 01, 2008
Larry, I am not familiar with the usage of the second and third arguments in your call to string.Format(). Aren't those rectangular quantities graphical in nature and not textual? In other words, seems you want string lengths not graphical dimensions for those arguments. TwainAnonymous
August 01, 2008
The comment has been removedAnonymous
August 01, 2008
You didn't select a font into the memory DC. So... unless you plan to draw with the default font (ugh), you're gonna get incorrect results.Anonymous
August 01, 2008
Isn't 'string' a keyword of standard C++?Anonymous
August 01, 2008
I notice that you are not checking to make sure that CreateCompatibleDC returns a valid DC. I'm not 100% sure that the application has a current screen before you've issued the ShowWindow() call, but it probably does. The more confusing thing is that you are using a variable named string which may cause problems with the STL libraries if you are using any of those. An issue that you can sometimes run into is because each element in your RECT is declared as a LONG, the simple math you do in that format string is done with LONGs. The format string itself is declaring that your output is %d which is just "int" and so the numbers might and up being strange in your output. Either typecasting the math result to "int" or using the format string %ld would take care of the particular issue.Anonymous
August 01, 2008
That's probably not it, but 2 things:
- You most likely want to set a nicer font than the default
- DT_CENTER looks unnecessary
Anonymous
August 01, 2008
Martin, the window visible thing isn't relevant, the code as written will exhibit the bug (I did check). William Bonner: The code dos return a valid DC, since it's a memory DC As for the string thing, you're right, I was sloppy. Leo: The "What's wrong with this code" posts are all about pointing out gotchas that aren't always obvious, usually after I've spent a long time trying to figure something out. They're interactive, which allows people to comment (and to point out my mistakes as several people have above). Ivo: DT_CENTER might not be necessary, I'm not sure. But when I go ahead and actually draw the text, it will have the flag, so I added it. And you're <i>very</i> close on the "nicer font" part of your comment. The challenge is realizing why chosing a nicer font matters.Anonymous
August 01, 2008
Doh! That must be it - you need to select the correct font with the correct size, orientation, etc. Unless you really want to use the default font/size. And I second Shog9's "ugh" on this.Anonymous
August 01, 2008
Ivo: Why is setting the correct size important? And I'd missed Shog9's comment, thanks for pointing it out. Remember, I tried this code and it worked perfectly. But while I was testing the change, I discovered that I had made a horrible but really subtle mistake.Anonymous
August 01, 2008
From MSDN on CreateCompatibleDC: "When a memory DC is created, all attributes are set to normal default values." Default font is SYSTEM_FONT, which is not considered "nice" these days :). I believe that adding (as a simplest example) SelectObject(hDC, GetStockObject(DEFAULT_GUI_FONT)) before DrawText(hDC, ...) would illustrate the problem that Larry is referring to.Anonymous
August 01, 2008
Konstantin: In my case, selecting DEFAULT_GUI_FONT actually made things worse :). The font's the right answer, but there's something more going on - what scenario did I test that nobody so far has mentioned?Anonymous
August 01, 2008
"what scenario did I test that nobody so far has mentioned?" The scenario where the default font is meant to be written right-to-left?Anonymous
August 01, 2008
Perhaps using "Large fonts"? Or changing the system's default GUI font in another fashion?Anonymous
August 01, 2008
Something to do with right-to-left fonts maybe? Just guessing, I have no idea :)Anonymous
August 01, 2008
The comment has been removedAnonymous
August 01, 2008
Jon, good thought, but that would be symetrical.Anonymous
August 01, 2008
> what scenario did I test that nobody so far has mentioned? Large fonts? Does SYSTEM_FONT not scale properly?Anonymous
August 01, 2008
Ding!Ding!Ding! Eldan nailed it: Large fonts. For whatever reason, when you set the system to 144 DPI (large fonts), the default font that's used for the control I'm using to draw the text gets dramatically larger than either the system font OR the default GUI font. Which caused my size calculations to be wrong, which caused some ugly clipping.Anonymous
August 01, 2008
Also Justin: Apparently the system font doesn't scale the same as other fonts do.Anonymous
August 02, 2008
While we are talking volume controls. Could you explain why it's only possible to lower the volume in Windows (i.e. setting a volume between 0% and 100%) and not raise it (i.e setting it higher than 100%)?Anonymous
August 02, 2008
Nils: Way off topic. I'll answer in another post.Anonymous
August 02, 2008
Ummm... Why not use GetTextExtentPoint32 [ http://msdn.microsoft.com/en-us/library/ms534223(VS.85).aspx ]? "The GetTextExtentPoint32 function computes the width and height of the specified string of text."
- Oli
Anonymous
August 02, 2008
Oli: Because GetTextExtentPoint32 doesn't do word wrap, and for my case, I need the word wrap behavior.Anonymous
August 02, 2008
The comment has been removedAnonymous
August 02, 2008
The comment has been removedAnonymous
August 02, 2008
Larry, completely off topic, but since you're toying with volume-control related UI now, maybe you can fix this bug that really annoys me every time I open the mixer in vista/server2k8: I have my default window colour set to light gray as I find it much easier on the eyes than the default white. When I open the mixer, the application names for each of the per-application volume controls all have black text on white backgrounds, rather than using the default window background colour. See http://www.dhiren.za.net/files/pictures/mixer.pngAnonymous
August 03, 2008
The amount of software that gets broken with large fonts is evidence to something terribly rotten in the API (including it's documentation in this context). The API merely providing means to achieve the goal of extent calculation simply isn't enough; it should lead the developer away from such pitfalls. Oh well, I guess this can be attributed to backward compatibility or legacy code or whatever.Anonymous
August 03, 2008
Now that 3 or 4 bugs in the code have been pointed out, perhaps the original post can be updated to reflect the fixes? i would hate for someone googling for how to measure text to find this on MSDN and thinks it's the perferred method.Anonymous
August 03, 2008
Ian: Typically I post an answers link with the corrected code in it. Dhiren: You know, this is the first that anyone's pointed it out to me... I'll file a bug to make sure it's fixed at some point in the future :). Eldan: It's more complicated than that. My code was incorrect from the beginning, I just hadn't tested it enough. Large fonts happened to point out the fundamental bug in my code, but the bug was there all along - it's entirely possible the bug would have been found with an international version of the code for example.Anonymous
August 03, 2008
The comment has been removedAnonymous
August 03, 2008
Nils Arthur asked: why it's only possible to lower the volume in Windows (i.e. setting a volume between 0% and 100%) and not raise it (i.e setting it higher than 100%)? Because it doesn't go up to eleven ?Anonymous
August 04, 2008
Leo Davidson, I think the comments here prove your point. I agree with you.Anonymous
August 04, 2008
Indeed large fonts is just a single facet of the problem. The whole font-handling segment of the GDI API is problematic. That bug which have been there all along wouldn't have been there if the API would guide you to obtain the correct font before you measure.Anonymous
August 04, 2008
This is without running the code, just by reading:
- Isn't there a conflict between using DT_END_ELLIPSIS (which changes the text it does not fit in the rectangle) together with DT_CALCRECT (which expands the rectangle to fit the text)?
- The original rectangle is 88 pixels wide. And then we check the behavior of DT_CALCRECT: The rectangle gets expanded vertically and the text is wrapped, unless the largest word is wider than the rectangle. In that case the width gets increased to fit the largest word. So at big DPI the 88 pixels are too little and the width will grow.
- Using generic APIs and data types (MessageBox, CAtlString, etc.) with Unicode strings (L"...") instead of generic text ( _T("...") or TEXT("...") ). Probably does not matter in Win, I assume it's all build as Unicode (and is detected at compile time, and has nothing to do with the font :-)
Anonymous
August 04, 2008
Mihai, actually the wordwrap and endelipsis flags together mean that it won't actually add the elipsis until it has to. From what I've seen, the API doesn't change the width.Anonymous
August 04, 2008
The other day, I wrote about measuring the dimensions of a piece of text using the DrawText API . My