Texture warping broken since texture refactor

Forum rules
Please don't bump threads here if you have a problem - it will often be forgotten about if you do. Instead, make a new thread here.

Post a reply

Smilies
:D :) :( :o :shock: :? 8-) :lol: :x :P :oops: :cry: :evil: :twisted: :roll: :wink: :geek: :ugeek: :!: :?: :idea: :arrow: :| :mrgreen: :3: :wub: >:( :blergh:
View more smilies

BBCode is OFF
Smilies are ON

Topic review
   

Expand view Topic review: Texture warping broken since texture refactor

Re: Texture warping broken since texture refactor

by Rachael » Tue Sep 24, 2019 9:54 am

I've merged the patch as thus.

With one caveat: While yes, I am confident in the code, and I know it fixes the underlying issue, I know it still could be made better, as discussed in this thread. I did a squash commit so that if it's decided to redo this a completely different way, it's easier to revert 1 commit than 4.

At any rate, as far as my involvement goes, this issue is fixed.

Re: Texture warping broken since texture refactor

by drfrag » Wed Sep 18, 2019 11:34 am

Now i've noticed that it only crashed and glitched with mipmaps enabled, so it was that. :)

Re: Texture warping broken since texture refactor

by Rachael » Wed Sep 18, 2019 11:05 am

The "* 2" could be reduced to "* 4/3" to be honest - that's to hold the mipmaps.

You'll crash without it, otherwise, if you try to run this in the truecolor renderer with mipmaps enabled.

"hq_texture_hqresizemult" tells the texture resizer how big you want your textures. The value has to be squared when allocating for a warp texture.

Re: Texture warping broken since texture refactor

by drfrag » Wed Sep 18, 2019 10:43 am

Hey i still don't see it, gl_texture_hqresizemult by default is 1 and why the *2 for the Resize? :?

Re: Texture warping broken since texture refactor

by drfrag » Wed Sep 18, 2019 10:29 am

Now i've noticed, at first glance they looked identical.

Re: Texture warping broken since texture refactor

by Rachael » Wed Sep 18, 2019 10:25 am

No, it is not an exact duplicate, and the reason the code is duplicated is because it accesses a member variable of FTextureWarp that is not available in FSoftwareTexture. Both functions should probably be rewritten as one with a variable pointer parameter instead, but that's something someone can worry about later. (if they care enough and they have enough time)

Re: Texture warping broken since texture refactor

by drfrag » Wed Sep 18, 2019 10:14 am

Thanks for the explanation. I mostly understood what the code does, i had no idea about the Gentime tough.
The *4 was a blind shot, i thought that a pixel in truecolor takes 4 times the space as in paletted. Then i wanted to see if it crashed with less and i put +1 (i think that actually doubles the size) but it was not a fix. I was surprised that Softpoly apparently worked well. Also i thought that HQ resize modes were disabled by default, i never would have thought about that. :)
Edit: i see that the new FWarpTexture::GenerateBgraMipmapsFast() is identical to FSoftwareTexture::GenerateBgraMipmapsFast(), can't you just use that one?

Re: Texture warping broken since texture refactor

by Rachael » Wed Sep 18, 2019 9:28 am

https://github.com/madame-rachelle/gzdo ... ee91fc2771

Commit with mipmaps fix (I think?). It should be in the pull request already, too.

If this looks good, I'll go ahead and merge it.

Re: Texture warping broken since texture refactor

by Rachael » Wed Sep 18, 2019 7:40 am

I don't think you really understand what the individual lines are actually doing.

The texture data is being accessed from a source texture and a new, temporary, software-only texture is created for the warp. If a texture is supposed to be warped, then the renderer will call the texture's "WarpTexture::GetPixels" method, which calls the functions you have modified, depending on whether truecolor mode is active or not.

Code: Select all

		auto otherpix = FSoftwareTexture::GetPixelsBgra();
This stores a reference to the source texture in its original form in a variable called 'otherpix'

Code: Select all

		WarpedPixelsRgba.Resize(GetWidth() * GetHeight());
The name here is a bit of a misnomer. WarpedPixelsRgba is just a TArray which is GZDoom's standard array type. Resizing simply means it is reallocating the buffer. Without this - you get the crashes that were mentioned earlier in this thread. This line in its unmodified form does not account for the larger textures that are created in the resize, hence the crashing issue.

The reason it works when you do "+1" or some other crazy bullshit like that is because a program's data space is allocated in chunks - so there was enough space for the extra garbage even though it technically was not actually allocated. This would be like if a file on a disk used its extra wasted sector space, which even though the sectors are allocated, the individual bytes are not in use.

As you can guess, putting "+ 1" is a hugely improper fix to this solution and you should not do that. Always allocate your arrays to the proper size that you actually need!

Code: Select all

		WarpBuffer(WarpedPixelsRgba.Data(), otherpix, GetWidth(), GetHeight(), WidthOffsetMultiplier, HeightOffsetMultiplier, time, mTexture->shaderspeed, bWarped);
This executes the actual texture warp on the new texture being created, using the 'otherpix' texture that was referenced earlier. In the patch this is fixed so that GetWidth() and GetHeight() are set to the proper size, that way the entire texture is warped, and is warped in the proper (resized) dimensions.

Code: Select all

		FreeAllSpans();
I'm not 100% sure about this, but I would reasonably guess that all this does is unallocate some now unused texture spans.

Code: Select all

		GenTime[2] = time;
This simply sets the check at the start of this code block so that it doesn't execute until the next frame. This is important because you only want to generate a warp texture once per frame; not once per pixel.

Re: Texture warping broken since texture refactor

by drfrag » Wed Sep 18, 2019 7:22 am

I see, i've tried only E1M1.

Re: Texture warping broken since texture refactor

by Rachael » Wed Sep 18, 2019 7:20 am

No, that is very much not the case. Running a test case myself and I got a garbled texture in the 8-bit renderer without the patch I created.

Re: Texture warping broken since texture refactor

by drfrag » Wed Sep 18, 2019 7:14 am

Really? I'm completely lost here. The 8 bit renderer already worked well, AFAIK Carmack 32 bit truecolor is the only one affected.
Even changing that line to 'WarpedPixelsRgba.Resize(GetWidth() * GetHeight() + 1);' prevents the crash.
Edit: i've even tried to fill the buffer with 'WarpBuffer(WarpedPixelsRgba.Data(), otherpix, GetWidth()*2, GetHeight()*2, WidthOffsetMultiplier, HeightOffsetMultiplier, time, mTexture->shaderspeed, bWarped);' and it's better but there are artifacts.

Re: Texture warping broken since texture refactor

by Rachael » Wed Sep 18, 2019 7:05 am

Incomplete work put into a pull request.

This fixes the problem completely in the classic 8-bit renderer, however the truecolor renderer is missing mipmaps and must be completed before this can be accepted. This is unfortunately where I am lost.

Re: Texture warping broken since texture refactor

by Rachael » Wed Sep 18, 2019 6:54 am

Thank you, drfrag! That gave me some ideas on how to fix this. I am testing the fix now before pushing, but it looks like we've pretty much got this.

Re: Texture warping broken since texture refactor

by drfrag » Wed Sep 18, 2019 5:10 am

Woah! Some "progress", i expected a mighty crash but now i get a more "interesting" effect and it doesn't crash. :P
Screenshot_Doom_20190918_130212.png

Code: Select all

--- a/src/rendering/swrenderer/textures/warptexture.cpp
+++ b/src/rendering/swrenderer/textures/warptexture.cpp
@@ -60,7 +60,7 @@ const uint32_t *FWarpTexture::GetPixelsBgra()
 	if (time != GenTime[2])
 	{
 		auto otherpix = FSoftwareTexture::GetPixelsBgra();
-		WarpedPixelsRgba.Resize(GetWidth() * GetHeight());
+		WarpedPixelsRgba.Resize(GetWidth() * GetHeight() * 4);
 		WarpBuffer(WarpedPixelsRgba.Data(), otherpix, GetWidth(), GetHeight(), WidthOffsetMultiplier, HeightOffsetMultiplier, time, mTexture->shaderspeed, bWarped);
 		FreeAllSpans();
 		GenTime[2] = time;

Top