View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0002891 | FSSCP | scripting | public | 2013-06-08 16:22 | 2014-06-13 08:35 |
Reporter | FelixJim | Assigned To | m_m | ||
Priority | normal | Severity | minor | Reproducibility | always |
Status | resolved | Resolution | fixed | ||
OS | Windows 7 | ||||
Product Version | 3.7.0 RC2 | ||||
Fixed in Version | 3.7.2 RC3 | ||||
Summary | 0002891: gr.drawString text box height irregular | ||||
Description | gr.drawString has five arguments: (string, x1, y1, x2, y2). x1 and y1 correctly control where the top left hand corner is placed, and x2 correctly controls the the position of the right hand border of the text box. However, the effect of y2 on the text box height seems highly irregular - at very small differences between y1 and y2 chunks of the string are carved off an unpredictable fashion, and larger (but can still be fairly small) differences appear to have no effect on the string. | ||||
Steps To Reproduce | Complete test mod is attached, with in-mission instructions. Observe how as you lower the text box height (shown visually as a rectangle with the same coordinates), nothing happens until you get very close to a height of 0, where very small steps can cause large chunks of the string to disappear. | ||||
Additional Information | Decreasing the text box width too low to show the string causes a debug error; presumably to explain to the modder what's happened to their string. No such error observed when the height gets too low. | ||||
Tags | No tags attached. | ||||
|
|
|
I have a patch which fixes the describes issue. I have no idea what the original code was supposed to do but I hope that I got the intended behavior right. I also fixed another bug which caused the text to skip one line right at the beginning of the text. |
|
Would fixing that second bug cause all uses of drawString up until now to be shifted up a line? |
|
No, only the ones that are using the string bounding mechanism. |
|
I see you had issues with lowercase min() and resorted to std::min(), after undef'ing min Can you not instead use our macro MIN() - note capitalisation - instead and not have to undef? |
|
I would suggest that we simply define NOMINMAX (http://support.microsoft.com/kb/143208) for windows builds which would suppress the definition of these macros. These definitions have caused nothing but issues and aren't used anywhere in the code so removing them seems to be the way to go. What do you think about that approach? |
|
mantis2891.patch (1,847 bytes)
Index: code/parse/lua.cpp =================================================================== --- code/parse/lua.cpp (revision 10795) +++ code/parse/lua.cpp (working copy) @@ -13723,6 +13723,11 @@ return ADE_RETURN_NIL; } +// Stupid windows headers +#ifdef min +#undef min +#endif + #define MAX_TEXT_LINES 256 static char *BooleanValues[] = {"False", "True"}; static const int NextDrawStringPosInitial[] = {0, 0}; @@ -13782,21 +13787,25 @@ int linelengths[MAX_TEXT_LINES]; const char *linestarts[MAX_TEXT_LINES]; + if (y2 >= 0 && y2 < y) + { + // Invalid y2 value + Warning(LOCATION, "Illegal y2 value passed to drawString. Got %d y2 value but %d for y.", y2, y); + + int temp = y; + y = y2; + y2 = temp; + } + num_lines = split_str(s, x2-x, linelengths, linestarts, MAX_TEXT_LINES); //Make sure we don't go over size int line_ht = gr_get_font_height(); - y2 = line_ht * (y2-y); - if(y2 < num_lines) - num_lines = y2; + num_lines = MIN(num_lines, (y2 - y) / line_ht); - y2 = y; - + int curr_y = y; for(int i = 0; i < num_lines; i++) { - //Increment line height - y2 += line_ht; - //Contrary to WMC's previous comment, let's make a new string each line int len = linelengths[i]; char *buf = new char[len+1]; @@ -13804,13 +13813,23 @@ buf[len] = '\0'; //Draw the string - gr_string(x,y2,buf,GR_RESIZE_NONE); + gr_string(x,curr_y,buf,GR_RESIZE_NONE); //Free the string we made delete[] buf; + + //Increment line height + curr_y += line_ht; } - - NextDrawStringPos[1] = y2+gr_get_font_height(); + + if (num_lines <= 0) + { + // If no line was drawn then we need to add one so the next line is + // aligned right + curr_y += line_ht; + } + + NextDrawStringPos[1] = curr_y; } return ade_set_error(L, "i", num_lines); } |
|
New patch attached which applies cleanly to current trunk. |
|
Fix committed to trunk@10796. |
Date Modified | Username | Field | Change |
---|---|---|---|
2013-06-08 16:22 | FelixJim | New Issue | |
2013-06-08 16:22 | FelixJim | File Added: drawStringtest.7z | |
2013-06-22 19:39 | m_m | Note Added: 0015137 | |
2013-06-22 19:39 | m_m | Assigned To | => m_m |
2013-06-22 19:39 | m_m | Status | new => code review |
2013-06-22 19:40 | m_m | File Added: mantis2891.patch | |
2013-06-22 20:04 | FelixJim | Note Added: 0015138 | |
2013-06-22 20:07 | m_m | Note Added: 0015139 | |
2013-06-23 00:18 | Echelon9 | Note Added: 0015141 | |
2013-06-23 06:41 | m_m | Note Added: 0015142 | |
2014-06-12 15:19 | m_m | File Deleted: mantis2891.patch | |
2014-06-12 15:19 | m_m | File Added: mantis2891.patch | |
2014-06-12 15:19 | m_m | Note Added: 0015846 | |
2014-06-13 08:35 | m_m | Note Added: 0015854 | |
2014-06-13 08:35 | m_m | Status | code review => resolved |
2014-06-13 08:35 | m_m | Fixed in Version | => 3.7.2 RC3 |
2014-06-13 08:35 | m_m | Resolution | open => fixed |