Source Rect does not work properly [FIXED] - RESOLVED

#1 I had a quick look at this for you and I believe the problem is that the source rectangles type is incorrectly specified internally in its AddProtectedField definition.

"mSrcRect" is a "RectI" whereas its type is being specified as "TypeRectF" in both the static-sprite and scroller code.

addProtectedField("sourceRect", TypeRectF, Offset(mSrcRect, t2dScroller), &setSourceRect, &defaultProtectedGetFn, "");

addProtectedField("sourceRect", TypeRectF, Offset(mSrcRect, t2dStaticSprite), &setSourceRect, &defaultProtectedGetFn, "");

Both the types above should be "TypeRectI" as far as I can see.

Also, be aware that the source rectangle stuff isn't hooked-up to the binary serialize or copy-to methods (cloning).

I did not develop this code so I have just logged this as a bug.

#2 Hi Melv, thanks for the reply. I tried to fix this by myself but I didn't reached a complete solution: I will wait for TGB 1.7.5 SP1 ^^

#3 Just change the "TypeRectF" for "TypeRectI" above and it should work.

#4 TGB-490

#5 Update: read post #7 for more fixing!

Melv "super keen eye" May strikes again, many thanks!

Using the change you suggested the editor works fine, at least regarding t2dStaticSprite.

About t2dScroller, the issue is more extended. The editor does not allow you to change the Use SourceRect property neither the Source Rect text field at all: they remains always blank values.

I digged inside the code and I noticed setSourceRect console method is present for t2dScroller, but it lacks other three console methods (getSourceRect, setUseSourceRect and getUseSourceRect).

To fix t2dScroller, make the following changes to t2dScroller.cc:

1) Add this at the top of the file, just after #include "./t2dScroller.h"

#include "core/stringBuffer.h"

2) Change TypeRectF to TypeRectI

addProtectedField("sourceRect", TypeRectI, Offset(mSrcRect, t2dScroller), &setSourceRect, &defaultProtectedGetFn, "");

3) Delete this

//----------------------------------------------------------------
// Set SourceRect
//----------------------------------------------------------------
ConsoleMethod( t2dScroller, setSourceRect, void, 6, 6, "(S32 x, S32 y, S32 w, S32 h) Set the source rect for the object")
{
	if(argc == 6)
	{
		RectI	sRect(dAtoi(argv[2]), dAtoi(argv[3]), dAtoi(argv[4]), dAtoi(argv[5]));
		object->setSourceRect(&sRect);
	}
	else
	{
		object->setSourceRect(NULL);
	}
}

4) Add this instead

//-------------------------------------------------------------------------
// Source rect controls
//-------------------------------------------------------------------------

ConsoleMethod(t2dScroller, getUseSourceRect, const char*, 2, 2, "() - Gets whether the source rects are used in this sprite.n"
															   "@return (bool) YES or NO")
{
    //Return correct values
	if(object->getUseSourceRect())
	{
		return "YES";
	}
	else
	{
		return "NO";
	}
}   

ConsoleMethod(t2dScroller, setUseSourceRect, void, 3, 3, "() - Sets whether the source rects are used in this sprite.n"
			  "@return (void))")
{
	if(dStrcmp("YES", argv[2]) == 0)
	{
		object->setUseSourceRect(true);
	}
	else
	{
		object->setUseSourceRect(false);
	}
}

//-----------------------------------------------------------------------------
// Get the source rect
//-----------------------------------------------------------------------------

ConsoleMethod(t2dScroller, getSpriteSourceRect, const char*, 2, 2, "() - Gets the source rect of this sprite.n"
															   "@return (four component rect) X Y Width Height")
{

	if(object->getUseSourceRect())
	{
		RectI source = object->getSourceRect();
		//Return correct values
		StringBuffer value;
		char topleft[4];
		char topright[4];
		char width[4];
		char height[4];

		dItoa(source.point.x, topleft);
		dItoa(source.point.y, topright);
		dItoa(source.extent.x, width);
		dItoa(source.extent.y, height);

		value.append(topleft);
		value.append(" ");
		value.append(topright);
		value.append(" ");
		value.append(width);
		value.append(" ");
		value.append(height);
		
		const char* values = value.createCopy8();
		return values;
	}
	else
	{
		return "0 0 0 0";
	}

	return "0 0 0 0";
}

//-----------------------------------------------------------------------------
// Set the source rect
//-----------------------------------------------------------------------------
ConsoleMethod( t2dScroller, setSpriteSourceRect, void, 3, 3, "(S32 x, S32 y, S32 w, S32 h) Set the source rect for the object")
{
	if(argc == 3)
	{
		F32 x,y,w,h;
		S32 args = dSscanf(argv[2], "%g %g %g %g", &x, &y, &w, &h);
		if(args == 4)
		{
			RectI tmp = RectI(x, y, w, h);
			object->setSourceRect(&tmp);
		}
	}
	else
	{
		object->setSourceRect(NULL);
	}
}

Now all should finally work. Remember to fix t2dStaticSprite.cc also with the TypeRectF to TypeRectI change.

There's eventually an incorrect warning message t2dStaticSprite :: Invalid Source Rect (xxxxxxImageMap) Ignoring (no rendering will happen!) also in the console.log file. It's due to this code line inside t2dStaticSprite.h:

void setUseSourceRect( bool bUseSourceRect ) { mUseSourceRect = bUseSourceRect; setSourceTextureCoords();}

Just after setting mUseSourceRect to true, setSourceTextureCoords() function checks if sourceRect is ok, but that moment it's still "0 0 0 0" so it's never ok the first time. t2dScroller does not call setSourceTextureCoords() just after setting UseSourceRect, so it does not throw that false warning.

#6 Yeah, I hadn't run-up TGB 1.7.5 for this so I didn't noticed any editor problems, I just did it by looking at the code. Looks like good fixes here though but it'd need to be verified.

I don't like the use of the "YES" and "NO" which was used in this code as it's non-standard. That should be changed to use "1"/"0" (dAtob) in the next release although that'll break anyones code which relies upon it.

I've logged this on TGB-490.

Good work Giuseppe.

#7 Hi guys,
the new source rect feature really needs heavy fixing! I hope this is because all GG efforts are towards T2D. ^^

I found an incorrect source rect settings usage inside t2dStaticSprite and t2dScroller. If you use imageMaps coming from POW textures, it all works fine, but in the other cases it does not work properly.

I.e. try creating a sprite with an imagemap from a 800x600 image, and setting its source rect to "0 0 800 600". You will see big white borders appearing around the sprite. Doing the same with a 1024x1024 image works well.

The issue is the way the new setSourceTextureCoords function calculates the source rect coordinates: it uses bitmap (instead of texture) width and height, resulting with incorrect final texture coordinates.

Inside the setSourceTextureCoords at t2dStaticSprite.cc, you should change this

F32	fTexelWidth = 1.0f / (F32)mImageMapDataBlock->getSrcBitmapWidth();
F32	fTexelHeight = 1.0f / (F32)mImageMapDataBlock->getSrcBitmapHeight();

with this

const TextureHandle& texture = mImageMapDataBlock->getImageMapFrameTexture( mFrame );
F32	fTexelWidth = 1.0f / texture.getWidth();
F32	fTexelHeight = 1.0f / texture.getHeight();

You need to change t2dScroller.cc also, just use 0 as mFrame value

const TextureHandle& texture = mImageMapDataBlock->getImageMapFrameTexture( 0 );
F32	fTexelWidth = 1.0f / texture.getWidth();
F32	fTexelHeight = 1.0f / texture.getHeight();

This the fastest way I found to fix this, but I think all this new source rect feature need a very good revision to make it a little more elegant.

#8 Thanks Giuseppe for the fixes. Just wanted to add a bit more after reading some of the code. I would rather recommend changing this part

if(object->getUseSourceRect())
{
   RectI source = object->getSourceRect();
   //Return correct values
   StringBuffer value;
   char topleft[4];
   char topright[4];
   char width[4];
   char height[4];

   dItoa(source.point.x, topleft);
   dItoa(source.point.y, topright);
   dItoa(source.extent.x, width);
   dItoa(source.extent.y, height);

   value.append(topleft);
   value.append(" ");
   value.append(topright);
   value.append(" ");
   value.append(width);
   value.append(" ");
   value.append(height);
		
   const char* values = value.createCopy8();
   return values;
}
else
{
   return "0 0 0 0";
}

return "0 0 0 0";

to

if(object->getUseSourceRect())
{
   RectI source = object->getSourceRect();
   StringBuffer value;

   dSprintf(buf, 1024, "%d %d %d %d", source.point.x, source.point.y, source.extent.x, source.extent.y);
}
else 
{
   dSprintf(buf, 1024, "0 0 0 0");
}

return buf;

#9 I found the problem in t2dStaticSprite was when setSourceRect() received a NULL parameter and it set an uninitialized RectI() in the local field. That is what produces the artifacts.

The real bug is that RectI() and RectF() are not initialized by default to \"0,0,0,0\".

#10 Fixed in 1.7.6.