2019-10-18 23:23 EDT


View Issue Details Jump to Notes ]
IDProjectCategoryView StatusLast Update
0002891FSSCPscriptingpublic2014-06-13 04:35
ReporterFelixJim 
Assigned Tom_m 
PrioritynormalSeverityminorReproducibilityalways
StatusresolvedResolutionfixed 
PlatformOSWindows 7OS Version
Product Version3.7.0 RC2 
Target VersionFixed in Version3.7.2 RC3 
Summary0002891: gr.drawString text box height irregular
Descriptiongr.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 ReproduceComplete 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 InformationDecreasing 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.
TagsNo tags attached.
Attached Files
  • 7z file icon drawStringtest.7z (2,380 bytes) 2013-06-08 12:22
  • patch file icon mantis2891.patch (1,847 bytes) 2014-06-12 11:19 -
    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);
     }
    
    patch file icon mantis2891.patch (1,847 bytes) 2014-06-12 11:19 +

-Relationships
+Relationships

-Notes

~0015137

m_m (developer)

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.

~0015138

FelixJim (reporter)

Would fixing that second bug cause all uses of drawString up until now to be shifted up a line?

~0015139

m_m (developer)

No, only the ones that are using the string bounding mechanism.

~0015141

Echelon9 (developer)

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?

~0015142

m_m (developer)

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?

~0015846

m_m (developer)

New patch attached which applies cleanly to current trunk.

~0015854

m_m (developer)

Fix committed to trunk@10796.
+Notes

-Issue History
Date Modified Username Field Change
2013-06-08 12:22 FelixJim New Issue
2013-06-08 12:22 FelixJim File Added: drawStringtest.7z
2013-06-22 15:39 m_m Note Added: 0015137
2013-06-22 15:39 m_m Assigned To => m_m
2013-06-22 15:39 m_m Status new => code review
2013-06-22 15:40 m_m File Added: mantis2891.patch
2013-06-22 16:04 FelixJim Note Added: 0015138
2013-06-22 16:07 m_m Note Added: 0015139
2013-06-22 20:18 Echelon9 Note Added: 0015141
2013-06-23 02:41 m_m Note Added: 0015142
2014-06-12 11:19 m_m File Deleted: mantis2891.patch
2014-06-12 11:19 m_m File Added: mantis2891.patch
2014-06-12 11:19 m_m Note Added: 0015846
2014-06-13 04:35 m_m Note Added: 0015854
2014-06-13 04:35 m_m Status code review => resolved
2014-06-13 04:35 m_m Fixed in Version => 3.7.2 RC3
2014-06-13 04:35 m_m Resolution open => fixed
+Issue History