new texture palette management bug

Post Reply
zeromus
Posts: 212
Joined: Wed Mar 31, 2010 6:05 pm

new texture palette management bug

Post by zeromus » Tue Aug 02, 2011 5:58 am

I have uncovered what I think is a bug in the new texture palette management. If you allocate some 2bpp textures, such that the next available address is halfway through the alignment block size for higher bitdepth textures, then the next allocated texture (of a higher bitdepth) will get allocated to the wrong alignment. Basically, it seems like the alignment code in vramBlock_examineSpecial does nothing useful. I've attached a patch which fixes my immediate issue. Whether it is safe or not... it would be nice if discostew could take a look. In some short time I will be checking in a live example of this into Paletted_Cube if you wish to wait for that. I just had to post this while it was fresh on my mind.
Attachments
palette_management.zip
(598 Bytes) Downloaded 419 times

Discostew
Posts: 103
Joined: Sun Mar 08, 2009 7:24 pm

Re: new texture palette management bug

Post by Discostew » Tue Aug 02, 2011 8:58 am

I believe I see the problem you're describing, and you are correct about the initial alignment code being rather useless for palettes. That's because of the static address being sent to it every time for palettes, which is VRAM_E, which pretty much bypasses the alignment test. Even if that was working, alignment could be lost later on during the "bank-lock examination" loop when moving from one available block to another, for which you seem to have a fix. I'll need to examine the overall code a bit more to ensure stability.

Thanks for finding a problem with my rather unreadable code. (I still need to make comments all over those functions) :P

Discostew
Posts: 103
Joined: Sun Mar 08, 2009 7:24 pm

Re: new texture palette management bug

Post by Discostew » Wed Aug 03, 2011 3:06 am

Ok, I took a deeper look into the situation along with your fix, and I don't think your solution will cause any problems. My main concern was with compressed textures because it requires having to examine 2 spots, but I don't think it'll be affected negatively. In fact, the problem you discovered would probably have affected compressed textures as well. Your fix handles both the initial alignment code I had (which only worked sometimes as I mentioned above) as well as when the address has to be changed in the later loop. My alignment code can safely be removed, and yours added in.

zeromus
Posts: 212
Joined: Wed Mar 31, 2010 6:05 pm

Re: new texture palette management bug

Post by zeromus » Wed Aug 03, 2011 4:12 am

there are still problems. after I cause the allocated address to get bumped up to the next alignment, subsequent textures get repeatedly loaded to that address. You'll see when I check in paletted cube. It is taking a while because it is part of a larger project to add texture support to grit.

if you want to take a stab at it, i uploaded the palette memory through 4 frames of emulation. these are snapped in order left to right frame by frame
Attachments
palettes.png
(23.57 KiB) Not downloaded yet

Discostew
Posts: 103
Joined: Sun Mar 08, 2009 7:24 pm

Re: new texture palette management bug

Post by Discostew » Wed Aug 03, 2011 8:07 am

Alright. I'll investigate it a bit further. I'll be gone on vacation next week, so I'll try to figure out the main cause before then.

Discostew
Posts: 103
Joined: Sun Mar 08, 2009 7:24 pm

Re: new texture palette management bug

Post by Discostew » Thu Aug 04, 2011 4:56 am

I found the root of this most recent problem with the palettes, but I should warn you. You might feel silly after seeing it. I know I did even after I felt your change was stable.

Code: Select all

mb->lastExamined = block;
mb->lastExaminedAddr = aligned_checkAddr;
mb->lastExaminedSize = size + excess;
return aligned_checkAddr;
See the problem? "size + excess"? The address might change due to alignment, but the size is not meant to change, as it is the length in bytes of the block to be allocated. This resulted in the "lastExaminedSize" being different than the "size" being rechecked later for the actual allocation, which then never happened because it exited soon after the check. The code kept going for loading the palette because it used a valid address upon returning from the code above. If the examination truly failed, the return address would have been NULL.

Anyways, removing that excess part seemed to fix the issue in my tests, but you can check it yourself also. This code tracing was needed, as prior to finding this exact problem, I stumbled across a few others, including a mistake in the last patch that I uploaded.

zeromus
Posts: 212
Joined: Wed Mar 31, 2010 6:05 pm

Re: new texture palette management bug

Post by zeromus » Thu Aug 04, 2011 5:57 am

sure enough, that fixes it for all my cases. to be honest, I did not analyze what values should be stuffed into mb. I still dont understand how it gets used, but I thought I should indicate that the entire block (from the last starting point, up to the padded point, and then to the end of the newly allocated block) was reserved for the new texture. I thought I had guessed every permutation of pointers and sizes, but it seems not.

if you've made any other changes, you should upload a final patch yourself so that we're sure we're using the same thing that will get checked in

Discostew
Posts: 103
Joined: Sun Mar 08, 2009 7:24 pm

Re: new texture palette management bug

Post by Discostew » Thu Aug 04, 2011 7:00 am

The whole allocation/deallocation process wasn't commented or very readable, so I wouldn't blame you for not understanding it. I even had to go through it again because I was trying to remember how it operated, and I was the one who made it. This time, however, I made a decent amount of comments while working on the fixes and improvements, so it'll be easier for people looking at it to understand the mechanics going on. The whole design would be comparable to a doubly-doubly linked list, if that is even a term. :lol:

I'm gonna go over it once more before I set it up as a patch. It's bad enough I left many holes unplugged it its development when I have pages and pages of design drawings sitting in my room of multiple allocation/deallocation scenarios.

Discostew
Posts: 103
Joined: Sun Mar 08, 2009 7:24 pm

Re: new texture palette management bug

Post by Discostew » Thu Aug 04, 2011 7:11 pm

The patch has been uploaded. Thanks for your help zeromus.

zeromus
Posts: 212
Joined: Wed Mar 31, 2010 6:05 pm

Re: new texture palette management bug

Post by zeromus » Sat Aug 06, 2011 3:35 am

I checked in the updated grit and paletted_cube if you want to tinker around with my examples for any further research

Post Reply

Who is online

Users browsing this forum: No registered users and 18 guests