Forums

Sega Master System / Mark III / Game Gear
SG-1000 / SC-3000 / SF-7000 / OMV
Home - Forums - Games - Scans - Maps - Cheats - Credits
Music - Videos - Development - Hacks - Translations - Homebrew

View topic - DevkitSMS noob: getting some garbled pixels when visualizing the background

Reply to topic Goto page 1, 2  Next
Author Message
  • Joined: 03 Dec 2021
  • Posts: 82
  • Location: Italy
Reply with quote
DevkitSMS noob: getting some garbled pixels when visualizing the background
Post Posted: Mon Jan 06, 2025 12:07 pm
Hi everyone,

I decided to give DevkitSMS a go and I started from sverx's amazing tutorial.
My first objective is to simply import a static background, but I decided to import a portion of the first map of Sonic the Hedgehog (I am attaching it here) instead of the image in the tutorial. I made sure the image is 16 colors, then I simply kept the same compression as those of the tutorial, namely "PSGaiden" for the tiles, "STM" for the tilemap and bin format for the palette using BMP2Tile. I am not sure this is the best choice but before changing anything other than the source image I wanted to test with these settings.

Then I ran these commands to create my sms ROM (output.sms):

python2.7 /home/umberto/games/gamedev/devkitSMS-master/assets2banks/src/assets2banks.py /home/umberto/games/gamedev/sms/cinos_2/assets
sdcc -c -mz80 bank2.c
sdcc -c -mz80 main.c
sdcc -o output.ihx -mz80 --data-loc 0xC000 --no-std-crt0 crt0_sms.rel main.rel SMSlib.lib bank2.rel
ihx2sms output.ihx output.sms


When I import the ROM file into Emolicious, this is what I am getting. As you can see, there are some garbled pixels around. Is this an issue of VRAM? Can you guys please tell me what could be the problem and how could I possibly fix it? I am planning to use even bigger images just so you know.

Thanks for your support and kudos for all the tools and this neat forum!!!


garbled_pixels.png (39.28 KB)
garbled_pixels.png

  View user's profile Send private message
  • Joined: 14 Apr 2013
  • Posts: 642
Reply with quote
Post Posted: Mon Jan 06, 2025 1:18 pm
umbe1987 wrote
Hi everyone,

I decided to give DevkitSMS a go and I started from sverx's amazing tutorial.
My first objective is to simply import a static background, but I decided to import a portion of the first map of Sonic the Hedgehog (I am attaching it here) instead of the image in the tutorial. I made sure the image is 16 colors, then I simply kept the same compression as those of the tutorial, namely "PSGaiden" for the tiles, "STM" for the tilemap and bin format for the palette using BMP2Tile. I am not sure this is the best choice but before changing anything other than the source image I wanted to test with these settings.

Then I ran these commands to create my sms ROM (output.sms):

python2.7 /home/umberto/games/gamedev/devkitSMS-master/assets2banks/src/assets2banks.py /home/umberto/games/gamedev/sms/cinos_2/assets
sdcc -c -mz80 bank2.c
sdcc -c -mz80 main.c
sdcc -o output.ihx -mz80 --data-loc 0xC000 --no-std-crt0 crt0_sms.rel main.rel SMSlib.lib bank2.rel
ihx2sms output.ihx output.sms


When I import the ROM file into Emolicious, this is what I am getting. As you can see, there are some garbled pixels around. Is this an issue of VRAM? Can you guys please tell me what could be the problem and how could I possibly fix it? I am planning to use even bigger images just so you know.

Thanks for your support and kudos for all the tools and this neat forum!!!


That looks like sprites from garbage data. Make sure to have the sprite delimiter at the start of the SAT.
There's probably some kind of initialization procedure for sprites in devkitSMS.
  View user's profile Send private message Visit poster's website
  • Joined: 05 Sep 2013
  • Posts: 4026
  • Location: Stockholm, Sweden
Reply with quote
Post Posted: Mon Jan 06, 2025 1:56 pm
@umbe1987 is your image larger than 32 tiles (256 pixels)? Images that are larger than the BG aren't supported directly, so please try again using a 256 pixel wide image.

@Calindro sprites should be 'automatically' initialized correctly by the library...
  View user's profile Send private message Visit poster's website
  • Joined: 03 Dec 2021
  • Posts: 82
  • Location: Italy
Reply with quote
Post Posted: Mon Jan 06, 2025 2:09 pm
sverx wrote
@umbe1987 is your image larger than 32 tiles (256 pixels)? Images that are larger than the BG aren't supported directly, so please try again using a 256 pixel wide image.


Hi @sverx. Yes, it is 768 pixels wide (three times bigger than the limit). How would I go about using larger image then? Can you or anyone else please point me to a resource I can try to follow to achieve what I need? After loading this image my second goal would be to add some horizontal scrolling so that I can see the whole image by pressing right and left.
  View user's profile Send private message
  • Joined: 03 Dec 2021
  • Posts: 82
  • Location: Italy
Reply with quote
Post Posted: Mon Jan 06, 2025 2:19 pm
umbe1987 wrote
sverx wrote
@umbe1987 is your image larger than 32 tiles (256 pixels)? Images that are larger than the BG aren't supported directly, so please try again using a 256 pixel wide image.


Hi @sverx. Yes, it is 768 pixels wide (three times bigger than the limit). How would I go about using larger image then? Can you or anyone else please point me to a resource I can try to follow to achieve what I need? After loading this image my second goal would be to add some horizontal scrolling so that I can see the whole image by pressing right and left.


Ok, I found the resource (farther in the same tutorial page). I will start from it ;)

https://www.smspower.org/forums/15888-DevkitSMSTutorial#91367
  View user's profile Send private message
  • Joined: 05 Sep 2013
  • Posts: 4026
  • Location: Stockholm, Sweden
Reply with quote
Post Posted: Mon Jan 06, 2025 3:25 pm
umbe1987 wrote
How would I go about using larger image then? Can you or anyone else please point me to a resource I can try to follow to achieve what I need? After loading this image my second goal would be to add some horizontal scrolling so that I can see the whole image by pressing right and left.


Unfortunately on the SMS the hardware background map is only 32 tiles wide (256 pixels) so when you want to scroll horizontally, you have to replace a whole column of tiles when one scrolls out of view. To do this, you probably also want to save your tilemaps in an uncompressed format, and very likely also in column-major order (instead of the common row-major order) so that you can later simply call
SMS_loadTileMapColumn(x,y,src,height)
to get the new column on screen.
  View user's profile Send private message Visit poster's website
  • Joined: 03 Dec 2021
  • Posts: 82
  • Location: Italy
Reply with quote
Post Posted: Mon Jan 06, 2025 7:08 pm
sverx wrote
To do this, you probably also want to save your tilemaps in an uncompressed format, and very likely also in column-major order (instead of the common row-major order) so that you can later simply call
SMS_loadTileMapColumn(x,y,src,height)
to get the new column on screen.

Thanks @sverx, that is very useful and super interesting! A few clarifications if I may: first, what is an uncompressed format (e.g. .inc?)? Second, how would I create a column-major tilemap? Do I have to modify the uncompressed tilemap generated by BMP2Tile myself (like transposing it) or is there an easier way?
  View user's profile Send private message
  • Site Admin
  • Joined: 19 Oct 1999
  • Posts: 15020
  • Location: London
Reply with quote
Post Posted: Mon Jan 06, 2025 8:54 pm
Bmp2tile will not compress if you save as .bin. It doesn’t do column major data though because you should expect to change your approach based on the level data format. For example, Sonic the Hedgehog levels are defined with one byte per 32x32 meta-tile, then some data defines the 16 graphics tiles used within each of these. Most games work similarly, with different sizes of meta-tile - 16x16 is very common, I’ve seen up to 96x96. So processing a PNG into raw tile data doesn’t make so much sense in this context. I just leave it to you to solve :)
  View user's profile Send private message Visit poster's website
  • Joined: 18 Jul 2020
  • Posts: 412
Reply with quote
Post Posted: Tue Jan 07, 2025 12:27 am
Last edited by xfixium on Tue Jan 07, 2025 9:28 am; edited 1 time in total
Nothing to see here, sverx's plugin is waaayyy more convenient XD
  View user's profile Send private message
  • Joined: 05 Sep 2013
  • Posts: 4026
  • Location: Stockholm, Sweden
Reply with quote
Post Posted: Tue Jan 07, 2025 8:51 am
umbe1987 wrote
how would I create a column-major tilemap? Do I have to modify the uncompressed tilemap generated by BMP2Tile myself (like transposing it) or is there an easier way?


You can either transpose the tilemap yourself or code a plugin for BMP2Tile or use the plugin I created already which you can find here, with its source code if needed.

or of course use this tool provided above by xfixium :D
  View user's profile Send private message Visit poster's website
  • Joined: 03 Dec 2021
  • Posts: 82
  • Location: Italy
Reply with quote
Post Posted: Tue Jan 07, 2025 1:11 pm
Thanks everyone! I now have plenty of good information I can exploit to move on. For now, I think I would be happy to test the column-major tilemap approach using @sverx's plugin for BMP2Tile and then try
SMS_loadTileMapColumn
function. If I succeed (and this would already be quite an achievement for me) I will evaluate the result and possibly consider meta-tiles and how I can approach them. Regardless, I will study more about meta-tiles, and I hope they are supported by DevkitSMS.

Also, @sverx, not that this is super important, but I just noticed that the
SMS_loadTileMapColumn
is not listed in https://github.com/sverx/devkitSMS/tree/master/SMSlib. It would be nice to add a bit more description to each functions in that page as well, so as to understand what the various arguments are supposed to represent. But everything is already very useful as it is now and I am grateful that it even exists :)
  View user's profile Send private message
  • Joined: 05 Sep 2013
  • Posts: 4026
  • Location: Stockholm, Sweden
Reply with quote
Post Posted: Tue Jan 07, 2025 2:11 pm
umbe1987 wrote
Regardless, I will study more about meta-tiles, and I hope they are supported by DevkitSMS.


They (currently) aren't. There are so many different ways this could be done, so maybe they will never be supported... but I also said something similar when we discussed metasprite support and later that was added anyway so... who knows!

umbe1987 wrote
but I just noticed that the
SMS_loadTileMapColumn
is not listed in https://github.com/sverx/devkitSMS/tree/master/SMSlib. It would be nice to add a bit more description to each functions in that page as well, so as to understand what the various arguments are supposed to represent.


Thanks for the heads-up, I always forget to update that file. It had a few other (minor) problems and some other functions were missing so I added them too. I'm not sure about writing a proper reference for every function/macro that exist, also because I think the SMSlib.h file is good enough at providing the needed info. But I'll think about it.
  View user's profile Send private message Visit poster's website
  • Joined: 03 Dec 2021
  • Posts: 82
  • Location: Italy
Reply with quote
Post Posted: Thu Jan 09, 2025 12:52 pm
Hello again!

@sverx thank you so much for providing the dll to export in column-major order :)

I have exported my image and now I am trying to use SMS_loadTileMapColumn. I am trying this code, but the output visually is a mess of garbled pixels. So I don't know exactly I am doing wrong here honestly... The image I am using is 368 by 192 pixels.


#include "SMSlib.h" // we're including the library with the functions we will use
#include "bank2.h"  // we're including the assets we created before

#define BG_TILES 0
#define BG_TILE_WIDTH 46
#define X_TILE_MAX 32
#define Y_TILE_MAX 24

void loadAssets(void)
{
    SMS_loadPSGaidencompressedTiles(field__tiles__psgcompr, BG_TILES);
    for (unsigned int y = 0; y < X_TILE_MAX; y++)
    {
        SMS_loadTileMapColumn(0, y, field__tilemap__cmraw,Y_TILE_MAX);
    }
    SMS_loadBGPalette(field__palette__bin);
}

void main(void)
{
    loadAssets();
    SMS_displayOn();
    for (;;)
        ;
}
  View user's profile Send private message
  • Joined: 05 Sep 2013
  • Posts: 4026
  • Location: Stockholm, Sweden
Reply with quote
Post Posted: Thu Jan 09, 2025 2:26 pm
umbe1987 wrote
I am trying this code, but the output visually is a mess of garbled pixels. So I don't know exactly I am doing wrong here honestly... The image I am using is 368 by 192 pixels.


You emulator will help you in understanding if the tiles are loaded correctly.

As for the tilemap, you're almost there, but you need to pass the source address for the data that needs to go into the column, for example like this:
SMS_loadTileMapColumn(x, 0, &field__tilemap__cmraw[x*Y_TILE_MAX],Y_TILE_MAX);

and also I believe you meant to loop using x, not y.
  View user's profile Send private message Visit poster's website
  • Joined: 03 Dec 2021
  • Posts: 82
  • Location: Italy
Reply with quote
Post Posted: Thu Jan 09, 2025 3:00 pm
Thank you so much @sverx, now it works!!!
Indeed, I was looping using the wrong coordinate :P ....
This is the working code and the image on screen (I had to insert a "*2" to the address to make it work).
It feels so good when it finally showed up!
Now on to the next steps...

#include "SMSlib.h" // we're including the library with the functions we will use
#include "bank2.h"  // we're including the assets we created before

#define BG_TILES 0
#define BG_TILE_WIDTH 46
#define X_TILE_MAX 32
#define Y_TILE_MAX 24

void loadAssets(void)
{
    SMS_loadPSGaidencompressedTiles(field__tiles__psgcompr, BG_TILES);
    for (unsigned int x = 0; x < X_TILE_MAX; x++)
    {
        SMS_loadTileMapColumn(x, 0, &field__tilemap__cmraw[x*Y_TILE_MAX*2],Y_TILE_MAX); // 32 tiles * 2 bytes each
    }
    SMS_loadBGPalette(field__palette__bin);
}

void main(void)
{
    loadAssets();
    SMS_displayOn();
    for (;;)
        ;
}

working.png (17.31 KB)
working.png

  View user's profile Send private message
  • Joined: 05 Sep 2013
  • Posts: 4026
  • Location: Stockholm, Sweden
Reply with quote
Post Posted: Fri Jan 10, 2025 9:22 am
umbe1987 wrote
This is the working code and the image on screen (I had to insert a "*2" to the address to make it work).


oh, that's because you're probably not declaring field__tilemap__cmraw as an unsigned int array. Are you using assets2banks?
  View user's profile Send private message Visit poster's website
  • Joined: 03 Dec 2021
  • Posts: 82
  • Location: Italy
Reply with quote
Post Posted: Fri Jan 10, 2025 10:43 am
sverx wrote
umbe1987 wrote
This is the working code and the image on screen (I had to insert a "*2" to the address to make it work).


oh, that's because you're probably not declaring field__tilemap__cmraw as an unsigned int array. Are you using assets2banks?


Yes I am using this command: python2.7 /home/umberto/games/gamedev/devkitSMS-master/assets2banks/src/assets2banks.py /home/umberto/games/gamedev/sms/cinos_2/assets
  View user's profile Send private message
  • Joined: 05 Sep 2013
  • Posts: 4026
  • Location: Stockholm, Sweden
Reply with quote
Post Posted: Fri Jan 10, 2025 12:57 pm
OK so, as you can see here, assets2banks can be configured to receive detailed instructions regarding how to process specific files.

If you create assets2banks.cfg (a text file) in your assets folder, then you can add these lines:
field (tilemap).cmraw
:format unsigned int

and the generated data will be in a unsigned int array, which corresponds to the fact that every entry in the map is a 16 bit value, and you won't need any ×2 anywhere to map the correct element of the array.

Using an assets2banks.cfg will also become useful later for a couple other things, trust me ;)
  View user's profile Send private message Visit poster's website
  • Joined: 03 Dec 2021
  • Posts: 82
  • Location: Italy
Reply with quote
Post Posted: Sun Jan 12, 2025 5:03 pm
sverx wrote
OK so, as you can see here, assets2banks can be configured to receive detailed instructions regarding how to process specific files.

If you create assets2banks.cfg (a text file) in your assets folder, then you can add these lines:
field (tilemap).cmraw
:format unsigned int

and the generated data will be in a unsigned int array, which corresponds to the fact that every entry in the map is a 16 bit value, and you won't need any ×2 anywhere to map the correct element of the array.

Using an assets2banks.cfg will also become useful later for a couple other things, trust me ;)


Quick (probably not very interesting) update on my progress. I have inserted the assets2banks.cfg to my assets to my assets folder so that now my code works without specifying the "*2" in my code as @sverx suggested.

I now have successfully implemented the horizontal scrolling towards the right until I reach I end of my map, and I am happy about it. Next thing i am going to implement will be scrolling to the left, and if I succeed I will try to add some vertical scrolling as well (probably).

I am more or less at the same point I reached some years ago using assembly, but it took me much less effort with DevkitSMS, so I am glad it exists! Part of this is also that I am probably a bit more knowledgeable on what I am supposed to do, but mostly it's because I understand C much more than Z80 assembly :)

I will try to post some more updates as I progress if you don't mind, or at least will write if I get stuck again. Side note: I have also setup Emulicious debugger with VScode and an automatic build task, so now I am a bit more fast in checking what's going on when I hit a wall.

Thanks again everyone for all your precious advice, I really appreciate them!
  View user's profile Send private message
  • Joined: 03 Dec 2021
  • Posts: 82
  • Location: Italy
Reply with quote
Post Posted: Wed Jan 15, 2025 9:41 pm
This might be a stupid question and I am pretty sure I must be doing something wrong which is very obvious but I cannot figure it out.

I am trying to use this ternary operator, but it never evaluates to true and changes my `bg_addr` value, what is wrong with that?

bg_addr = changeDir == 1 ? bg_addr - TILEMAP_WIDTH : bg_addr - 1;


TILEMAP_WIDTH is defined like this at the beginning of my script:

#define TILEMAP_WIDTH 32


changeDir is set initially as:


unsigned int changeDir = 0;



Before line 84 `bg_addr` is 81, after this it still is 81, but `changeDir` is 1 as you can see from the wathced variables in the debugger. It should become 81-32=42, which is `bg_addr - TILEMAP_WIDTH` but it stays unchanged. Surprisingly (for me) even the other expression does not do anything (81-1=80)....


ternary.png (164.9 KB)
ternary.png

  View user's profile Send private message
  • Joined: 18 Jul 2020
  • Posts: 412
Reply with quote
Post Posted: Thu Jan 16, 2025 2:26 am
The results that you are watching may be confusing due to optimizations that SDCC may have done when it compiled your code. This is mentioned in the "Unexpected Behavior In C Debugging" section of the VSCode Emulicious Debugger extension.
  View user's profile Send private message
  • Joined: 03 Dec 2021
  • Posts: 82
  • Location: Italy
Reply with quote
Post Posted: Thu Jan 16, 2025 7:35 am
xfixium wrote
The results that you are watching may be confusing due to optimizations that SDCC may have done when it compiled your code. This is mentioned in the "Unexpected Behavior In C Debugging" section of the VSCode Emulicious Debugger extension.


Thanks @xifium, I will have a deeper look into it when I can. What I observed yesterday so far while stepping into debugging is that all variables were changing as expected except for this particular step.
  View user's profile Send private message
  • Joined: 05 Sep 2013
  • Posts: 4026
  • Location: Stockholm, Sweden
Reply with quote
Post Posted: Thu Jan 16, 2025 9:08 am
umbe1987 wrote
I am trying to use this ternary operator, but it never evaluates to true and changes my `bg_addr` value, what is wrong with that?
bg_addr = changeDir == 1 ? bg_addr - TILEMAP_WIDTH : bg_addr - 1;


Honestly, I also often get into troubles using the ternary operator.
Sometimes just rewriting it with a bunch of parenthesis solve it - so, if you're sure that changeDir==1 can be both true or false, try with
bg_addr = ((changeDir == 1) ? (bg_addr - TILEMAP_WIDTH) : (bg_addr - 1));

edit: if that still fails, just use an if/then/else...
  View user's profile Send private message Visit poster's website
  • Joined: 03 Dec 2021
  • Posts: 82
  • Location: Italy
Reply with quote
Post Posted: Thu Jan 16, 2025 9:21 am
sverx wrote
umbe1987 wrote
I am trying to use this ternary operator, but it never evaluates to true and changes my `bg_addr` value, what is wrong with that?
bg_addr = changeDir == 1 ? bg_addr - TILEMAP_WIDTH : bg_addr - 1;


Honestly, I also often get into troubles using the ternary operator.
Sometimes just rewriting it with a bunch of parenthesis solve it - so, if you're sure that changeDir==1 can be both true or false, try with
bg_addr = ((changeDir == 1) ? (bg_addr - TILEMAP_WIDTH) : (bg_addr - 1));

edit: if that still fails, just use an if/then/else...


Thanks @sverx, that is good advice and I will surely try that as soon as I can. I also played a bit with parentheses myself yesterday as I thought that could fix the problem too, but it did not. I will come back to this when I can and possibly avoid the ternary operator to see how it goes.

By the way, can we use type _Bool or import sdbool.h with sdcc? I haven't tried it yet and for now I am keeping my changeDir variable as an unsigned integer, making sure its value is either 1 or 0, but I woul gladly do that if I can.
  View user's profile Send private message
  • Joined: 05 Sep 2013
  • Posts: 4026
  • Location: Stockholm, Sweden
Reply with quote
Post Posted: Thu Jan 16, 2025 9:23 am
umbe1987 wrote
By the way, can we use type _Bool or import sdbool.h with sdcc? I haven't tried it yet and for now I am keeping my changeDir variable as an unsigned integer, making sure its value is either 1 or 0, but I woul gladly do that if I can.


Sure, just put
#include <stdbool.h>
on top in your main.c :)
  View user's profile Send private message Visit poster's website
  • Joined: 06 Mar 2022
  • Posts: 709
  • Location: London, UK
Reply with quote
Post Posted: Thu Jan 16, 2025 10:12 am
umbe1987 wrote
I also played a bit with parentheses myself yesterday as I thought that could fix the problem too, but it did not. I will come back to this when I can and possibly avoid the ternary operator to see how it goes.

I think your ternary operators look fine as written, ternary has a very low operator precedence in C which means that most of the time it works as intended, but I do agree with sverx that ternaries can result in particularly illegible code which we don't have full confidence in, hence the resort to parentheses.

One other advantage that writing if statements might bring to this is that stepping in the debugger might (although this is not guaranteed) reveal something more useful to you. For example, you write:


tilemap_addr = timemap_addr > 0 ? tilemap_addr : TILEMAP_WIDTH;


which is equivalent to:

if (tilemap_addr <= 0)
  tilemap_addr = TILEMAP_WIDTH;


This formulation might allow the debugger to either step to the line within the if statement, or skip it entirely, revealing something more than the ternary might.

You could refactor your other ternary similarly, with an if / else which might again result in the debugger stepping more obviously between the two branches.

As to your original problem, if bg_addr genuinely is unchanged but line 84 is being executed, then it seems to me that TILEMAP_WIDTH may be evaluating to 0 rather than 32 as per your #define directive, so worth checking the position of the #define. Could you perhaps have another #define directive elsewhere which redefines TILEMAP_WIDTH to 0 (or e.g. 256 if you're working with bytes)?

P.S. 81-32=49, not 42, just to make sure you're expecting the right result here, but since you say that bg_addr isn't changing at all I guess that's not the problem!
  View user's profile Send private message Visit poster's website
  • Joined: 03 Dec 2021
  • Posts: 82
  • Location: Italy
Reply with quote
Post Posted: Thu Jan 16, 2025 10:33 am
Hi @willbriton, glad to hear from you after some years!

I agree with your statements about the ternary operators, I will try to switch to the normal if statement as see how it goes.

Quote
As to your original problem, if bg_addr genuinely is unchanged but line 84 is being executed, then it seems to me that TILEMAP_WIDTH may be evaluating to 0 rather than 32 as per your #define directive, so worth checking the position of the #define. Could you perhaps have another #define directive elsewhere which redefines TILEMAP_WIDTH to 0 (or e.g. 256 if you're working with bytes)?


Worth checking this indeed, although I think I am not redefining it anywhere, but it's better for me to make sure this is the case!

Quote
P.S. 81-32=49, not 42, just to make sure you're expecting the right result here, but since you say that bg_addr isn't changing at all I guess that's not the problem!


oops, yep I meant 49, that was a typo :)

Thanks again to everybody for the many suggestions! I will keep you posted when I come back to this.
  View user's profile Send private message
  • Joined: 05 Sep 2013
  • Posts: 4026
  • Location: Stockholm, Sweden
Reply with quote
Post Posted: Thu Jan 16, 2025 12:53 pm
willbritton wrote
I think your ternary operators look fine as written, ternary has a very low operator precedence in C which means that most of the time it works as intended, but I do agree with sverx that ternaries can result in particularly illegible code which we don't have full confidence in, hence the resort to parentheses.


<OT>
It might be just me, but I believe 50% of the times I struggle in understanding exactly why something isn't working as expected, I find I had used a ternary without enough parentheses there... :|
</OT>
  View user's profile Send private message Visit poster's website
  • Joined: 06 Mar 2022
  • Posts: 709
  • Location: London, UK
Reply with quote
Post Posted: Thu Jan 16, 2025 2:24 pm
sverx wrote
willbritton wrote
I think your ternary operators look fine as written, ternary has a very low operator precedence in C which means that most of the time it works as intended, but I do agree with sverx that ternaries can result in particularly illegible code which we don't have full confidence in, hence the resort to parentheses.


<OT>
It might be just me, but I believe 50% of the times I struggle in understanding exactly why something isn't working as expected, I find I had used a ternary without enough parentheses there... :|
</OT>


Oh absolutely!
In particular because the precedence is so low it causes significant issues when you try to use a ternary as part of a larger expression, compounded by C's weak typing.

Example:

  int foo = -2;
  int bar = -5;

  // prints 3...
  printf("%d\n", foo * bar > 0 ? foo - bar : foo + bar);
  // ...because equivalent to:
  printf("%d\n", ((foo * bar) > 0) ? (foo - bar) : (foo + bar));

  // prints -7:
  // (and actually raises a compiler warning in some compilers!)
  printf("%d\n", foo * (bar > 0) ? foo - bar : foo + bar);

  // prints 14:
  printf("%d\n", foo * (bar > 0 ? foo - bar : foo + bar));

  // prints -1:
  printf("%d\n", foo * (bar > 0 ? foo - bar : foo) + bar);

  // prints -2:
  printf("%d\n", (foo * bar > 0 ? foo - bar : foo) + bar);
  View user's profile Send private message Visit poster's website
  • Joined: 05 Sep 2013
  • Posts: 4026
  • Location: Stockholm, Sweden
Reply with quote
Post Posted: Fri Jan 17, 2025 8:45 am
willbritton wrote
In particular because the precedence is so low it causes significant issues when you try to use a ternary as part of a larger expression, compounded by C's weak typing.


Yes, most of the times I fall into this trap I had written something like
some_func (param, param2, SOME_VALUE + var != 0 ? 1 : 2)
instead of
some_func (param, param2, SOME_VALUE + (var != 0 ? 1 : 2) )
or similar.

Also, back to the topic, note @umbe1987 that you can avoid the multiplication by TILEMAP_HEIGHT (even if it isn't so expensive, being a multiplication by a constant) by using another variable to keep track of the source address for the next column.
Just something to keep in mind when you want to write C code that performs better (faster).
  View user's profile Send private message Visit poster's website
  • Joined: 03 Dec 2021
  • Posts: 82
  • Location: Italy
Reply with quote
Post Posted: Fri Jan 17, 2025 10:22 am
Quote
Also, back to the topic, note @umbe1987 that you can avoid the multiplication by TILEMAP_HEIGHT (even if it isn't so expensive, being a multiplication by a constant) by using another variable to keep track of the source address for the next column.
Just something to keep in mind when you want to write C code that performs better (faster).


Thanks @sverx I will do that!

BTW, I got rid of the bloody ternary operator and the expression is evaluated as it should now.
I am very close to a fully right and left horizontal scrolling now, just need to fine tune some edge cases. Hopefully I will have it working soon!
  View user's profile Send private message
  • Joined: 03 Dec 2021
  • Posts: 82
  • Location: Italy
Reply with quote
Post Posted: Sun Jan 19, 2025 3:42 pm
I am struggling to implement a bug-free horizontal scrolling. When everything seems to work pretty fine, I try to press right and left to stress test the code and only sometimes I experience graphical issues on screen, such as the wrong tile places in the wrong column.

It only happens sometimes, and even debugging has becoming difficult since it would take me forever to pinpoint when the issue happens.

I am sharing the code I made so far in the hope that someone could possibly chip in and spot any possible issue.

#include "SMSlib.h" // we're including the library with the functions we will use
#include "bank2.h"  // we're including the assets we created before

#define BG_TILES 0
#define BG_WIDTH 96
#define BG_HEIGHT 28
#define TILEMAP_WIDTH 32
#define TILEMAP_HEIGHT 28
#define SPRITE_TILES 256

unsigned int scroll = 0;
unsigned char playerPosition[2] = {(TILEMAP_WIDTH * 8) / 2, (TILEMAP_HEIGHT * 8) / 2};

void loadAssets(void)
{
    SMS_loadPSGaidencompressedTiles(field__tiles__psgcompr, BG_TILES);
    for (int i; i < TILEMAP_WIDTH; i++)
        SMS_loadTileMapColumn(i, 0, &field__tilemap__cmraw[i * TILEMAP_HEIGHT], TILEMAP_HEIGHT);
    SMS_loadBGPalette(field__palette__bin);
    SMS_loadPSGaidencompressedTiles(sprites__tiles__psgcompr, SPRITE_TILES);
    SMS_loadSpritePalette(sprites__palette__bin);
}

void drawPlayer(void)
{
    SMS_addSprite(playerPosition[0], playerPosition[1], SPRITE_TILES);
    SMS_addSprite(playerPosition[0] + 8, playerPosition[1], SPRITE_TILES + 2);
    SMS_addSprite(playerPosition[0] + 16, playerPosition[1], SPRITE_TILES + 4);
    SMS_addSprite(playerPosition[0], playerPosition[1] + 16, SPRITE_TILES + 6);
    SMS_addSprite(playerPosition[0] + 8, playerPosition[1] + 16, SPRITE_TILES + 8);
    SMS_addSprite(playerPosition[0] + 16, playerPosition[1] + 16, SPRITE_TILES + 10);
}

void main(void)
{
    unsigned int ks;
    SMS_setSpriteMode(SPRITEMODE_TALL);
    loadAssets();
    SMS_VDPturnOnFeature(VDPFEATURE_LEFTCOLBLANK); // hide the leftmost column
    SMS_displayOn();
    for (;;)
    {
        SMS_initSprites();
        drawPlayer();
        SMS_waitForVBlank();
        SMS_finalizeSprites();
        SMS_copySpritestoSAT();
        ks = SMS_getKeysStatus();
        SMS_loadTileMapColumn(0, 0, &field__tilemap__cmraw[TILEMAP_WIDTH * TILEMAP_HEIGHT], TILEMAP_HEIGHT);
        if ((ks & PORT_A_KEY_RIGHT) && (scroll < (BG_WIDTH - TILEMAP_WIDTH) * 8 - 1))
        {
            scroll++;
            SMS_setBGScrollX(-scroll);
            if ((scroll % 8) == 0)
            {
                SMS_loadTileMapColumn((TILEMAP_WIDTH + (scroll / 8)) % TILEMAP_WIDTH, 0, &field__tilemap__cmraw[(TILEMAP_WIDTH + (scroll / 8)) * TILEMAP_HEIGHT], TILEMAP_HEIGHT);
            }
        }
        if ((ks & PORT_A_KEY_LEFT) && (scroll > 0))
        {
            scroll--;
            SMS_setBGScrollX(-scroll);
            if ((scroll % 8) == 0)
            {
                SMS_loadTileMapColumn((TILEMAP_WIDTH + (scroll / 8)) % TILEMAP_WIDTH, 0, &field__tilemap__cmraw[(scroll / 8) * TILEMAP_HEIGHT], TILEMAP_HEIGHT);
            }
        }
        if (scroll == 0)
        {
            SMS_loadTileMapColumn(0, 0, &field__tilemap__cmraw[TILEMAP_WIDTH * TILEMAP_HEIGHT], TILEMAP_HEIGHT);
        }
    }
  View user's profile Send private message
  • Joined: 06 Mar 2022
  • Posts: 709
  • Location: London, UK
Reply with quote
Post Posted: Sun Jan 19, 2025 4:01 pm
At first glance, I'm not sure your left-bound logic is quite right.

When you move right, from scroll value 7 to 8 say, you're going to load one new column in to the right of the scroll window. But when you move left, say back from scroll value 8 to 7, you don't reload the previous column, because the test is when scroll % 8 == 0.

But I'm not sure about this, scrolling is such a head-f&*k, that's why scrolling libraries are a thing :)

Side note: your final if clause means that if you are scrolled all the way to the left you are needlessly reloading the column every frame.
  View user's profile Send private message Visit poster's website
  • Joined: 03 Dec 2021
  • Posts: 82
  • Location: Italy
Reply with quote
Post Posted: Sun Jan 19, 2025 4:34 pm
willbritton wrote
At first glance, I'm not sure your left-bound logic is quite right.

When you move right, from scroll value 7 to 8 say, you're going to load one new column in to the right of the scroll window. But when you move left, say back from scroll value 8 to 7, you don't reload the previous column, because the test is when scroll % 8 == 0.

But I'm not sure about this, scrolling is such a head-f&*k, that's why scrolling libraries are a thing :)

Side note: your final if clause means that if you are scrolled all the way to the left you are needlessly reloading the column every frame.


Hi @willbritton and thanks for the feedback!

The final if statement is indeed redundant and I will move it inside the left scroll routine so it is called only when it's needed.

For the rest, I really don't know where the problem is. I will try to put the compiled rom and the code in a repository soon so anybody can see how it works. It basically works fine-ish expect for some edge cases which I am struggling to figure out :/
  View user's profile Send private message
  • Joined: 03 Dec 2021
  • Posts: 82
  • Location: Italy
Reply with quote
Post Posted: Sun Jan 19, 2025 4:41 pm
willbritton wrote
But when you move left, say back from scroll value 8 to 7, you don't reload the previous column, because the test is when scroll % 8 == 0.


Mmm this is really a good hint, will have a look into this for sure, thanks!!!!
  View user's profile Send private message
  • Joined: 05 Sep 2013
  • Posts: 4026
  • Location: Stockholm, Sweden
Reply with quote
Post Posted: Mon Jan 20, 2025 8:38 am
There are a few different possible options regarding *when* to load a new column of tiles - my favorite is 'only when needed' which means to load it only if it's going to be visible, which is different from your approach.

In my approach, when (scroll % 8) becomes 0 you *don't* need a new column, as it means you're only seeing exactly 31 columns on screen - it's when you move away from that situation that you need a new column.

Since you're moving only 1 pixel each time, I would do that:
- if right (or left) is pressed, check if current scroll % 8 is 0, if it is then you need to load a new column, then increment (or decrement) your scroll variable, otherwise you don't need a new column and you simply increment (or decrement) your scroll variable.

Which column you need to load, and where on screen, it's quite simple math as every 8 pixels is a new tile and every 32 columns you have to get back filling column 0 on screen.
  View user's profile Send private message Visit poster's website
  • Joined: 03 Dec 2021
  • Posts: 82
  • Location: Italy
Reply with quote
Post Posted: Mon Jan 20, 2025 9:37 am
sverx wrote
There are a few different possible options regarding *when* to load a new column of tiles - my favorite is 'only when needed' which means to load it only if it's going to be visible, which is different from your approach.

In my approach, when (scroll % 8) becomes 0 you *don't* need a new column, as it means you're only seeing exactly 31 columns on screen - it's when you move away from that situation that you need a new column.

Since you're moving only 1 pixel each time, I would do that:
- if right (or left) is pressed, check if current scroll % 8 is 0, if it is then you need to load a new column, then increment (or decrement) your scroll variable, otherwise you don't need a new column and you simply increment (or decrement) your scroll variable.

Which column you need to load, and where on screen, it's quite simple math as every 8 pixels is a new tile and every 32 columns you have to get back filling column 0 on screen.


Hi @sverx! thanks for your feedback, I really appreciate your (and others) help! :)

I am already doing this (indeed, I have taken most inspiration from your vertical scrolling example in the devkitSMS tutorial post at https://www.smspower.org/forums/15888-DevkitSMSTutorial#91367).

However, what @willbriton pointed out might be the cause of my issue (I still need to check this though), namely that if I am in the middle of this 8 window back and forth movement, the logic might be buggy, and I might be calculating the wrong address when I change direction "mid-tile" (if this makes sense).

The code I am using is pasted in my previous comment (here at https://www.smspower.org/forums/20430-DevkitSMSNoobGettingSomeGarbledPixelsWhenV... I will try to upload the code and the compiled ROM when I can to a repository so it would be easier to let anyone reproduce the bug.

Basically, it kind of works most of the times, expect sometimes when changing direction back and forth, with one single column placed in the wrong address only when this direction change happens, but not as you keep moving towards the same direction.
  View user's profile Send private message
  • Site Admin
  • Joined: 19 Oct 1999
  • Posts: 15020
  • Location: London
Reply with quote
Post Posted: Mon Jan 20, 2025 9:52 am
Is it possibly also triggered if you press both directions at once when the scroll position is a multiple of 8?
  View user's profile Send private message Visit poster's website
  • Joined: 03 Dec 2021
  • Posts: 82
  • Location: Italy
Reply with quote
Post Posted: Mon Jan 20, 2025 9:59 am
Maxim wrote
Is it possibly also triggered if you press both directions at once when the scroll position is a multiple of 8?


Hi @maxim.

That is also a nice guess. I need to check this when i have time! Would you recommend checking against this possibility?

I just checked the original Sonic, and it seems if you press both, RIGHT has precedence (even if you are going LEFT and then you simultaneously press RIGHT later on, Sonic goes to the RIGHT).
  View user's profile Send private message
  • Joined: 05 Sep 2013
  • Posts: 4026
  • Location: Stockholm, Sweden
Reply with quote
Post Posted: Mon Jan 20, 2025 10:01 am
umbe1987 wrote
I am already doing this (indeed, I have taken most inspiration from your vertical scrolling example in the devkitSMS tutorial post at https://www.smspower.org/forums/15888-DevkitSMSTutorial#91367).


Oh, I am not doing that there, that's quite an old code. Also, as Maxim suggested, you might have tripped onto a different problem. See that also my old code there have the check for the other direction into an else branch, to ensure it won't try to move both ways in the same frame.
  View user's profile Send private message Visit poster's website
  • Joined: 06 Mar 2022
  • Posts: 709
  • Location: London, UK
Reply with quote
Post Posted: Mon Jan 20, 2025 10:08 am
I also wondered about the potential for an issue with both directions being pressed at once, due to lack of else clause, but I think I'd discounted it since it looks like the code does a "complete" scroll to the right followed by a "complete" scroll to the left and so would be logically like a non operation, except that as I noted before I don't think the left scroll and right scroll are proper inverses of each other.

Of course, there might be potential timing issues trying to do two column loads in a single frame, even if the logic were correct.
  View user's profile Send private message Visit poster's website
  • Joined: 03 Dec 2021
  • Posts: 82
  • Location: Italy
Reply with quote
Post Posted: Mon Jan 20, 2025 10:32 am
willbritton wrote
I also wondered about the potential for an issue with both directions being pressed at once, due to lack of else clause, but I think I'd discounted it since it looks like the code does a "complete" scroll to the right followed by a "complete" scroll to the left and so would be logically like a non operation, except that as I noted before I don't think the left scroll and right scroll are proper inverses of each other.

Of course, there might be potential timing issues trying to do two column loads in a single frame, even if the logic were correct.


I can confirm that even after fixing the "simultaneous pressing" of left and right (by implementing the else logic as @sverx did in his old example) the bug is still there, and I am pretty confident what @willbritton said is causing it :)

I will try to figure out how to fix it (either by adding a new condition to my "scroll % 8 == 0" left move routine, or by using a variable when the direction changes so I can also use it for the right routine, I still have to think what's better, maybe none of these :) )
  View user's profile Send private message
  • Joined: 06 Mar 2022
  • Posts: 709
  • Location: London, UK
Reply with quote
Post Posted: Mon Jan 20, 2025 10:50 am
I think sverx's suggested amendment should be relatively simple and not necessitate new conditions or variables: currently you do either scroll++ or scroll-- before you check to see if scroll % 8 == 0, but if you basically did the check for scroll % 8 == 0 first, then updated scroll at some point afterwards.
  View user's profile Send private message Visit poster's website
  • Joined: 03 Dec 2021
  • Posts: 82
  • Location: Italy
Reply with quote
Post Posted: Mon Jan 20, 2025 11:39 am
THANK YOU SO MUCH GUYS!!!

I have been having nightmares about this :) It is now working!!!

Next step is to increase the speed (which I have already experimented and it's not simply a matter of increasing/decreasing the scroll by a constant probably due to the scroll % 8 == 0 check).

Anyway, the working code is down here, thanks again for all the patience and precious support!

#include "SMSlib.h" // we're including the library with the functions we will use
#include "bank2.h"  // we're including the assets we created before

#define BG_TILES 0
#define BG_WIDTH 96
#define BG_HEIGHT 28
#define TILEMAP_WIDTH 32
#define TILEMAP_HEIGHT 28
#define SPRITE_TILES 256

unsigned int scroll = 0;
unsigned char playerPosition[2] = {(TILEMAP_WIDTH * 8) / 2, (TILEMAP_HEIGHT * 8) / 2};

void loadAssets(void)
{
    SMS_loadPSGaidencompressedTiles(field__tiles__psgcompr, BG_TILES);
    for (int i; i < TILEMAP_WIDTH; i++)
        SMS_loadTileMapColumn(i, 0, &field__tilemap__cmraw[i * TILEMAP_HEIGHT], TILEMAP_HEIGHT);
    SMS_loadBGPalette(field__palette__bin);
    SMS_loadPSGaidencompressedTiles(sprites__tiles__psgcompr, SPRITE_TILES);
    SMS_loadSpritePalette(sprites__palette__bin);
}

void drawPlayer(void)
{
    SMS_addSprite(playerPosition[0], playerPosition[1], SPRITE_TILES);
    SMS_addSprite(playerPosition[0] + 8, playerPosition[1], SPRITE_TILES + 2);
    SMS_addSprite(playerPosition[0] + 16, playerPosition[1], SPRITE_TILES + 4);
    SMS_addSprite(playerPosition[0], playerPosition[1] + 16, SPRITE_TILES + 6);
    SMS_addSprite(playerPosition[0] + 8, playerPosition[1] + 16, SPRITE_TILES + 8);
    SMS_addSprite(playerPosition[0] + 16, playerPosition[1] + 16, SPRITE_TILES + 10);
}

void main(void)
{
    unsigned int ks;
    SMS_setSpriteMode(SPRITEMODE_TALL);
    loadAssets();
    SMS_VDPturnOnFeature(VDPFEATURE_LEFTCOLBLANK); // hide the leftmost column
    SMS_displayOn();
    for (;;)
    {
        SMS_initSprites();
        drawPlayer();
        SMS_waitForVBlank();
        SMS_finalizeSprites();
        SMS_copySpritestoSAT();
        ks = SMS_getKeysStatus();
        if ((ks & PORT_A_KEY_RIGHT) && (scroll < (BG_WIDTH - TILEMAP_WIDTH) * 8 - 1))
        {
            if ((scroll % 8) == 0)
            {
                SMS_loadTileMapColumn((TILEMAP_WIDTH + (scroll / 8)) % TILEMAP_WIDTH, 0, &field__tilemap__cmraw[(TILEMAP_WIDTH + (scroll / 8)) * TILEMAP_HEIGHT], TILEMAP_HEIGHT);
            }
            scroll ++;
            SMS_setBGScrollX(-scroll);
        }
        else if ((ks & PORT_A_KEY_LEFT) && (scroll > 0))
        {
            if ((scroll % 8) == 0)
            {
                SMS_loadTileMapColumn((TILEMAP_WIDTH + (scroll / 8)) % TILEMAP_WIDTH, 0, &field__tilemap__cmraw[(scroll / 8) * TILEMAP_HEIGHT], TILEMAP_HEIGHT);
            }
            scroll --;
            SMS_setBGScrollX(-scroll);
        }
    }
}
  View user's profile Send private message
  • Joined: 05 Sep 2013
  • Posts: 4026
  • Location: Stockholm, Sweden
Reply with quote
Post Posted: Mon Jan 20, 2025 2:44 pm
Happy you solved it!

umbe1987 wrote
Next step is to increase the speed (which I have already experimented and it's not simply a matter of increasing/decreasing the scroll by a constant probably due to the scroll % 8 == 0 check).


as long as you max horizontal speed doesn't get to 8 pixels/frame you could for instance check if the (old_scroll % 8) >= (new_scroll % 8) to see if you crossed a column when scrolling right, and something similar when scrolling left
  View user's profile Send private message Visit poster's website
  • Joined: 06 Mar 2022
  • Posts: 709
  • Location: London, UK
Reply with quote
Post Posted: Mon Jan 20, 2025 3:38 pm
Should those be divisions?
  View user's profile Send private message Visit poster's website
  • Joined: 03 Dec 2021
  • Posts: 82
  • Location: Italy
Reply with quote
Post Posted: Mon Jan 20, 2025 3:44 pm
willbritton wrote
Should those be divisions?


It's the modulo operation (a.k.a. remainder?) :)
  View user's profile Send private message
  • Joined: 05 Sep 2013
  • Posts: 4026
  • Location: Stockholm, Sweden
Reply with quote
Post Posted: Mon Jan 20, 2025 4:07 pm
willbritton wrote
Should those be divisions?


no, I really meant modulo.
When (new_scroll % 8) is not zero and it's < than what (old_scroll % 8) was, then it means you've crossed into a new column - sure you can also calculate that you're in a new column by using (new_scroll / 8) and comparing that to (old_scroll / 8) but you will likely have to check if (new_scroll % 8) is not zero anyway so I thought it was faster to use that.
  View user's profile Send private message Visit poster's website
  • Joined: 06 Mar 2022
  • Posts: 709
  • Location: London, UK
Reply with quote
Post Posted: Mon Jan 20, 2025 4:16 pm
sverx wrote
willbritton wrote
Should those be divisions?


no, I really meant modulo.
When (new_scroll % 8) is not zero and it's < than what (old_scroll % 8) was, then it means you've crossed into a new column - sure you can also calculate that you're in a new column by using (new_scroll / 8) and comparing that to (old_scroll / 8) but you will likely have to check if (new_scroll % 8) is not zero anyway so I thought it was faster to use that.

Ah, yeah sorry I was having a senior moment. Of course it works because left scroll will always make the fine pixel value smaller unless a column boundary is crossed, and vice-versa.

As you were everyone - I'm going for a nap!
  View user's profile Send private message Visit poster's website
  • Joined: 03 Dec 2021
  • Posts: 82
  • Location: Italy
Reply with quote
Post Posted: Mon Jan 20, 2025 5:49 pm
umbe1987 wrote
willbritton wrote
Should those be divisions?


It's the modulo operation (a.k.a. remainder?) :)


I understood the question now, so silly of me to reply LOL
  View user's profile Send private message
Reply to topic Goto page 1, 2  Next



Back to the top of this page

Back to SMS Power!