Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Possible imporvement to FadeStepColor #52

Open
tbsp opened this issue May 26, 2022 · 1 comment
Open

Possible imporvement to FadeStepColor #52

tbsp opened this issue May 26, 2022 · 1 comment

Comments

@tbsp
Copy link

tbsp commented May 26, 2022

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
Copy link

mhughson commented May 31, 2022

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants