Stephane-D / SGDK

SGDK - A free and open development kit for the Sega Mega Drive
https://www.patreon.com/SGDK
MIT License
1.75k stars 187 forks source link

Problem with VDP_drawImageEx() with image plane size 128x32 tiles #192

Closed a-dietrich closed 3 years ago

a-dietrich commented 3 years ago

I tried to use VDP_drawImageEx() to copy an image of 72x224 pixels into a large plane, which has been set to 128x32 tiles. Unfortunately, this seems to give a wrong result. It can be seen on the left of the attached image.

I did some digging, and in the SGDK CPU code path there might be a problem in setTileMapDataColumnEx(). The auto increment gets set to 2*128, which is then passed on as an u8. In the attached reproducer yoou can find a modified version of setTileMapDataColumnEx() that does not use the auto increment (block in the middle of the image).

Interestingly, when using a larger source image (e.g., 128x224 pixels) there is no problem. I've included a number of source images in the reproducer.

Thanks for having a look!

reproducer.zip

reproducer

Stephane-D commented 3 years ago

Oh indeed thanks for the report ! I never realized that we can't do tilemap column update when using the 128x32 tilemap size as the auto-increment register allow a value from 0 to 255 only (and 0 is actually 0, not 256...) That is a real failure in the design. I guess i will have to add checks to avoid that case to happen or at least put KLog messages to warn about it. I can explain why it's working using larger images: SGDK detects that tilemap row update would be more efficient here so it will use setTileMapDataRowEx(..) function instead which is not impacted by the issue.

Stephane-D commented 3 years ago

Fixed !

matteusbeus commented 3 years ago

Great work :D

From: Stephane Dallongeville notifications@github.com Sent: 25 February 2021 10:43 To: Stephane-D/SGDK SGDK@noreply.github.com Cc: Subscribed subscribed@noreply.github.com Subject: Re: [Stephane-D/SGDK] Problem with VDP_drawImageEx() with image plane size 128x32 tiles (#192)

Fixed !

- You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FStephane-D%2FSGDK%2Fissues%2F192%23issuecomment-785799361&data=04%7C01%7C%7C27196be7b4724c12b47508d8d97a1c64%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637498465767565662%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=zHwECfO3o3tuwovSxJOGpAPK%2F2IIu3PiKulKBPtj3dw%3D&reserved=0, or unsubscribehttps://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAIKSRRCAU7LJUQT2V3XLURLTAYSS7ANCNFSM4YFHXOUQ&data=04%7C01%7C%7C27196be7b4724c12b47508d8d97a1c64%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637498465767565662%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=73wmL6fiHEFlSGa%2B19XnmgBK2DZofLGPT57USN317D4%3D&reserved=0.

a-dietrich commented 3 years ago

Awesome, thanks!

Just tested with the master ...

reproducer