View Issue Details

IDProjectCategoryView StatusLast Update
0002891FSSCPscriptingpublic2014-06-13 08:35
ReporterFelixJim Assigned Tom_m  
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
OSWindows 7 
Product Version3.7.0 RC2 
Fixed 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.

Activities

FelixJim

2013-06-08 16:22

reporter  

drawStringtest.7z (2,380 bytes)

m_m

2013-06-22 19:39

developer   ~0015137

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.

FelixJim

2013-06-22 20:04

reporter   ~0015138

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

m_m

2013-06-22 20:07

developer   ~0015139

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

Echelon9

2013-06-23 00:18

developer   ~0015141

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?

m_m

2013-06-23 06:41

developer   ~0015142

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?

m_m

2014-06-12 15:19

developer  

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);
 }
mantis2891.patch (1,847 bytes)   

m_m

2014-06-12 15:19

developer   ~0015846

New patch attached which applies cleanly to current trunk.

m_m

2014-06-13 08:35

developer   ~0015854

Fix committed to trunk@10796.

Issue History

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