rsxSetTransferImage incorrect behavior
kakaroto opened this issue · 16 comments
Hi,
I don't know much (==nothing) about how the rsx nvidia command pipeline thingy stuff works, but I know there's a bug in rsxSetTransferImage. The issue is simple, if you want to copy a rect from the middle of one buffer to another using rsxSetTransferImage, it will always copy the buffer from its top-left corner rather than the rect that you specified.
Looking at the function in ppu/librsx/commands.c, you can clearly see that the srcX and srcY arguments to the function are completely ignored.
Please fix, thanks :)
Looking at the implementation of rsxSetTransferScaleSurface(), it seems the srcX and srcY should be inserted (as (srcY<<16)|srcX)instead of the 0 in the last word.
Yeah, it looks like it, that's what zerkman suggested, but when I tried it, it didn't work, maybe I didn't recompile it right or something. Either way, I don't have the code to test it right now, I'll try later and reconfirm whether or not it does fix it (it should from the looks of it).
I confirm the issue.
Also, after having patched commands.c with (srcY<<16)|srcX) as last word, the problem remains.
Hm. Maybe someone should check if the scale->inX and scale->inY parameters to rsxSetTransferScaleSurface() are broken too... :-/
Hey guys, I wrote that stuff so I'll take a look at it tonight.
I've just submitted a pull request that attempts to resolve this. Looking at source from the nouveau project, which I referred to when working out the DMA functions in the first place, it appears that the way to specify the rectangle's origin is to just add e.g. (srcY * srcPitch) + (srcX * bytesPerPixel) to srcOffset.
Does that really work for any srcX? Usually some amount of alignment is required for offsets...
I haven't tested it but I doubt it would work because I think that 'offset' is not a memory location, it's some random value (hash?) that the RSX assigns to a memory address, that's why you use rsxAddressToOffset even for rsx mapped memory. But, I might be wrong.
@zeldin, that's a good point, so I tested odd-numbered srcX and srcY passed to rsxSetTransferImage, which worked. However with a 4-byte pixel, that'd end up being 32-bit aligned offset anyway. I'll try testing a 1- and 2-byte per pixel blit to a texture or something soon (right now I'm blitting to the framebuffer, which must be formatted to 4-bytes per pixel no matter what).
gzorin, not sure that offset arithmetics is a good way of doing things.
however, here is the status of what I noticed:
- inX and inY are in fixed point format, or said another way, they measure 16ths of pixels. those are in the same format for rsxSetTransferScaleSurface().
- there is another problem, which is that if you use the blit width and height as source width and height, only the (0,0)-(width,height) source rectangle is copied, and the rest of the result is filled with black pixels. This can be solved by using the full source image width and height as source width and height.
- unfortunately, the rsxSetTransferImage call doesn't allow to specify the size of the source image. I suggest to deal with this by "generating" working witdth and height by using ceil(inX)+width and ceil(inY)+height.
I pushed the fixes to my own repo. It contains a "blitting" sample, with some 2D animation using the blit functions which work as expected.
However, the RSX seems to hang when exiting the program. I'm investigating about that, and once everything is fixed I'll push everything to the main repo.
Hmm. I guess I should read my notifications in chronological order instead of reversed. Feel free to unmerge that change and put in your fix instead.
Reverted. Carry on. :)
Ok, I finally pushed my fixes and a samples which illustrates how the blit functions can be used.
Nice! So you found the reason for the hang at exit then?
yes, I just forgot to wait for the flip before quitting.