Zal0 / ZGB

Game Boy / Color engine with lots of features
MIT License
703 stars 51 forks source link

Possible imporvement to FadeStepColor #52

Open tbsp opened 2 years ago

tbsp commented 2 years ago

I believe the FadeStepColor function could be improved by changing the calls to set_bkg_palette and set_sprite_palette to one call each outside the for loop:

https://github.com/Zal0/ZGB/blob/master/common/src/Fade_b.c#L61

void FadeStepColor(UINT8 i) {
    UINT8 pal, c;
    UWORD palette[4];
    UWORD palette_s[4];
    UWORD* col = ZGB_Fading_BPal;
    UWORD* col_s = ZGB_Fading_SPal;

    for(pal = 0; pal < 8; pal ++) {
        for(c = 0; c < 4; ++c, ++col, ++col_s) {
                palette[c] = UpdateColor(i, *col);
                palette_s[c] = UpdateColor(i, *col_s);
        };
    }
    set_bkg_palette(pal, 8, palette);
    set_sprite_palette(pal, 8, palette_s);
    delay(20);
}

This would reduce the number of function calls, and in cases where the for loop takes longer than a single frame would reduce the chances of the palette updates spanning frames (since they occur closer together), thus reducing out-of-sync palette updates.

mhughson commented 2 years ago

A think few additional changes are needed:

  1. palette and palette_s should be size of 4*8, rather than 4.
  2. Assignment of palette(_s) array should account for the size change.
  3. delay should be replaces with a vblank delay.
  4. Add a vblank delay to avoid mid screen changes.
void FadeStepColor(UINT8 i) {
    UINT8 pal, c;
    UWORD palette[4*8]; // change
    UWORD palette_s[4*8]; // change
    UWORD* col = ZGB_Fading_BPal;
    UWORD* col_s = ZGB_Fading_SPal;

    for(pal = 0; pal < 8; pal ++) {
        for(c = 0; c < 4; ++c, ++col, ++col_s) {
                palette[c + (pal*4)] = UpdateColor(i, *col); // change
                palette_s[c + (pal*4)] = UpdateColor(i, *col_s); // change
        };
    }

    // Wait for vblank so that we don't start the fade in the middle
    // of the screen.
    fade_vbl_delay(1); // change
    // Apply all 8 palettes at the exact same time.
    set_bkg_palette(0, 8, palette);
    set_sprite_palette(0, 8, palette_s);
    fade_vbl_delay(2); // change
}

Before:

image

After:

image