Skip to content

Commit c8d8166

Browse files
committed
Keep a dynamic shadow out of colour rotations
A shadow painted on the plane it belongs to never got a rotation entry, so it kept whatever an earlier frame or the allocation left there and could cycle through a rotation's colours. It now gets the same 'no rotation' entry as the dynamic content that casts it.
1 parent 50ec91f commit c8d8166

4 files changed

Lines changed: 214 additions & 7 deletions

File tree

‎AGENTS.md‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -945,6 +945,21 @@ not: a shadow pixel used to keep the entry belonging to the colour it had covere
945945
so `Serum_Rotate()` would animate the shadow with someone else's rotation. The
946946
replay stamps `0xffff`.
947947

948+
Outside layer mode `CheckDynaShadow()` paints the shadow in place, and it sets
949+
the rotation entry of the pixel to `0xffff` with it. It did not before. The
950+
pixel loop skips a pixel a shadow has claimed, so such a pixel kept the entry an
951+
earlier frame had left there, or what the allocation held on the first frame
952+
after a load, and a shadow painted over a pixel already rendered kept that
953+
pixel's entry. `Serum_Rotate()` then animated the shadow: on `afm_113b` frames
954+
16 to 77 (white high-score digits over a sky whose stars rotate through white)
955+
the shadow under every digit ran through the stars' colours. This path is every
956+
`32p`-only request for a 32-row ROM, and the native plane of the other two
957+
layouts.
958+
959+
**Rule: neither dynamic-zone content nor a dynamic shadow takes part in a colour
960+
rotation, even when its colour is one of the rotation's colours.** Both get the
961+
entry `0xffff`. Background, static and sprite pixels are looked up by colour.
962+
948963
The offset is a persisted per-colorization choice, `shadowOffsetMode`:
949964
`SERUM_SHADOW_OFFSET_NATIVE` (1 extra-plane pixel — what the pre-layer renderer
950965
produced, and what authors tuned against) or `_PROPORTIONAL` (2 pixels, matching

‎src/serum-decode.cpp‎

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5823,7 +5823,16 @@ static inline bool ColorInRotationOnPlane(bool planeRotates, uint32_t IDfound,
58235823
// sdShadowColour. isdynapix is left alone in that case -- the pixel is not
58245824
// dynamic content yet, and marking it would stop the static pass filling in
58255825
// what the shadow sits on.
5826-
void CheckDynaShadow(uint16_t* pfr, const uint8_t* shadowDirByLayer,
5826+
//
5827+
// prot: the per-pixel rotation entries of the plane pfr is. A shadow painted
5828+
// here is a flat colour and takes no part in a rotation, so its entry is set
5829+
// to 0xffff with it. The pixel loop does not do that: it skips a pixel a
5830+
// shadow has claimed, so the entry kept what an earlier frame had put there,
5831+
// or what the allocation held on the first frame, and Serum_Rotate() then
5832+
// animated the shadow. A shadow painted over a pixel the loop had already
5833+
// rendered kept that pixel's entry the same way.
5834+
void CheckDynaShadow(uint16_t* pfr, uint16_t* prot,
5835+
const uint8_t* shadowDirByLayer,
58275836
const uint16_t* shadowColorByLayer, uint8_t dynacouche,
58285837
uint8_t* isdynapix, uint16_t fx, uint16_t fy, uint32_t fw,
58295838
uint32_t fh, const uint8_t* shadowDirFallback = nullptr,
@@ -5857,6 +5866,10 @@ void CheckDynaShadow(uint16_t* pfr, const uint8_t* shadowDirByLayer,
58575866
}
58585867
isdynapix[neighborIndex] = 1;
58595868
pfr[neighborIndex] = tcol;
5869+
if (prot) {
5870+
prot[neighborIndex * 2] = 0xffff;
5871+
prot[neighborIndex * 2 + 1] = 0xffff;
5872+
}
58605873
}
58615874
}
58625875

@@ -6068,7 +6081,7 @@ void Colorize_Framev2(uint8_t* frame, uint32_t IDfound,
60686081
// picture the upscale reads has to be shadow-free for that to
60696082
// mean anything. ReplayOriginalPlaneShadows() puts it back
60706083
// afterwards for the 32p output.
6071-
CheckDynaShadow(pfr, frameShadowDir, frameShadowColor,
6084+
CheckDynaShadow(pfr, prot, frameShadowDir, frameShadowColor,
60726085
dynacouche, isdynapix, ti, tj,
60736086
g_serumData.fwidth, g_serumData.fheight,
60746087
shadowDirFallback, shadowColorFallback,
@@ -6279,9 +6292,9 @@ void Colorize_Framev2(uint8_t* frame, uint32_t IDfound,
62796292
}
62806293
}
62816294
if (!dynamicBlackSuppressed) {
6282-
CheckDynaShadow(pfr, frameShadowDirExtra, frameShadowColorExtra,
6283-
dynacouche, isdynapix, ti, tj,
6284-
g_serumData.fwidth_extra,
6295+
CheckDynaShadow(pfr, prot, frameShadowDirExtra,
6296+
frameShadowColorExtra, dynacouche, isdynapix,
6297+
ti, tj, g_serumData.fwidth_extra,
62856298
g_serumData.fheight_extra);
62866299
isdynapix[tk] = 1;
62876300
pfr[tk] = dynamicColor;

‎tests/README.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,9 @@ and a packing bug corrupts every colorization at once.
120120
| `rotation/advances_tagged_pixels_only` | a pixel tagged with a slot advances, a pixel tagged `0xffff` never does |
121121
| `rotation/wraps_at_length` | the shift wraps at the rotation's length instead of running off its colour list |
122122
| `rotation/empty_slot_not_due_next` | a slot with no colours or no delay is not a rotation and is not reported as due |
123+
| `rotation/dynamic_content_and_shadow_32p` | a dynamic pixel in a rotation's colour and the shadow it casts, above and below, have the entry `0xffff` and stand still while the background rotates; without the `0xffff` in `CheckDynaShadow()` the shadow keeps an old entry and rotates |
124+
| `rotation/dynamic_content_and_shadow_64p` | the same on the upscaled plane for a `64P`-only request |
125+
| `rotation/dynamic_content_and_shadow_both` | and for `32P \| 64P`, on both planes |
123126

124127
### `is256x64` projects
125128

‎tests/unit_tests.cpp‎

Lines changed: 178 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -505,8 +505,9 @@ static void Test_DeferredShadowLeavesThePlaneAlone(void) {
505505
std::vector<uint8_t> isdynapix((size_t)kW * kH, 0);
506506
mySerum.frame32[(size_t)6 * kW + 5] = 0xABCD; // what the shadow will cover
507507

508-
CheckDynaShadow(mySerum.frame32, dir.data(), col.data(), 0, isdynapix.data(),
509-
5, 5, kW, kH, nullptr, nullptr, /*defer=*/true);
508+
CheckDynaShadow(mySerum.frame32, mySerum.rotationsinframe32, dir.data(),
509+
col.data(), 0, isdynapix.data(), 5, 5, kW, kH, nullptr,
510+
nullptr, /*defer=*/true);
510511

511512
const size_t below = (size_t)6 * kW + 5;
512513
CHECK_EQ(mySerum.frame32[below], 0xABCD); // untouched, so it can be scaled
@@ -2041,6 +2042,175 @@ static void Test_EmptyRotationSlotIsNotDueNext(void) {
20412042
TearDownFrame();
20422043
}
20432044

2045+
// ---------------------------------------------------------------------------
2046+
// Rotations and dynamic content
2047+
//
2048+
// A rotating background (stars) under a dynamic zone whose glyph has a colour
2049+
// that is one of the rotation's colours, and a dynamic shadow above and below
2050+
// the glyph. Only the background moves: dynamic content is coloured from its
2051+
// set and a shadow is a flat colour, whatever colours the frame's rotations
2052+
// name. afm_113b frames 16 to 77 are this picture: white high-score digits
2053+
// over a sky whose stars rotate through white.
2054+
// ---------------------------------------------------------------------------
2055+
2056+
static const uint16_t kStarA = 0x0222, kStarB = 0x0333; // the rotation
2057+
static const uint16_t kGlyphShadow = 0x0444;
2058+
static const uint32_t kGlyphX = 5, kGlyphY = 6;
2059+
static const uint32_t kStarX = 20, kStarY = 20; // background, far from it
2060+
2061+
enum RotationRequest { kRequest32, kRequest64, kRequestBoth };
2062+
2063+
static void RenderGlyphOverRotatingBackground(RotationRequest request) {
2064+
SetUpColorization();
2065+
const size_t px = (size_t)kW * kCH;
2066+
const std::vector<uint16_t> bg(px, kStarA);
2067+
g_serumData.backgroundframes_v2.set(0, bg.data(), px);
2068+
const std::vector<uint8_t> mask(px, 1);
2069+
g_serumData.backgroundmask.set(kFrameId, mask.data(), px);
2070+
const uint16_t background[1] = {0};
2071+
g_serumData.backgroundIDs.set(kFrameId, background, 1);
2072+
2073+
// A 3x3 zone, lit in the middle only; the glyph takes the rotation's
2074+
// second colour from its set.
2075+
std::vector<uint8_t> active(px, 0);
2076+
for (uint32_t y = kGlyphY - 1; y <= kGlyphY + 1; ++y)
2077+
for (uint32_t x = kGlyphX - 1; x <= kGlyphX + 1; ++x)
2078+
active[(size_t)y * kW + x] = 1;
2079+
std::vector<uint16_t> shadeToColour(16, 0);
2080+
shadeToColour[5] = kStarB;
2081+
GiveFrameADynamicZone(active, /*couche=*/0, shadeToColour);
2082+
std::vector<uint8_t> dir(MAX_DYNA_SETS_PER_FRAME_V2, 0);
2083+
std::vector<uint16_t> col(MAX_DYNA_SETS_PER_FRAME_V2, 0);
2084+
// Above, a pixel the raster has rendered already, and below, one it has
2085+
// not reached yet.
2086+
dir[0] = (1u << 1) | (1u << 5);
2087+
col[0] = kGlyphShadow;
2088+
g_serumData.dynashadowsdir.set(kFrameId, dir.data(), dir.size());
2089+
g_serumData.dynashadowscol.set(kFrameId, col.data(), col.size());
2090+
2091+
const size_t tableSize = MAX_COLOR_ROTATION_V2 * MAX_LENGTH_COLOR_ROTATION;
2092+
std::vector<uint16_t> table(tableSize, 0);
2093+
table[0] = 2; // two colours
2094+
table[1] = 10; // every 10 ms
2095+
table[2] = kStarA;
2096+
table[3] = kStarB;
2097+
g_serumData.colorrotations_v2.reserve(tableSize);
2098+
g_serumData.colorrotations_v2_extra.reserve(tableSize);
2099+
g_serumData.colorrotations_v2.set(kFrameId, table.data(), tableSize);
2100+
g_serumData.BuildColorRotationLookup();
2101+
mySerum.rotations32 = (uint16_t*)calloc(tableSize, sizeof(uint16_t));
2102+
mySerum.rotations64 = (uint16_t*)calloc(tableSize, sizeof(uint16_t));
2103+
memcpy(mySerum.rotations32, table.data(), tableSize * sizeof(uint16_t));
2104+
memcpy(mySerum.rotations64, table.data(), tableSize * sizeof(uint16_t));
2105+
2106+
upscaleExtraFromOriginal = request != kRequest32;
2107+
originalPlaneRequestedByCaller = request != kRequest64;
2108+
// What an earlier frame left behind, or the allocation on the first frame:
2109+
// an entry nothing writes would name slot 0.
2110+
memset(mySerum.rotationsinframe32, 0, px * 2 * sizeof(uint16_t));
2111+
memset(mySerum.rotationsinframe64, 0, px * 8 * sizeof(uint16_t));
2112+
memset(colorshifts32, 0, sizeof(colorshifts32));
2113+
memset(colorshifts64, 0, sizeof(colorshifts64));
2114+
2115+
g_rom.assign(px, 0);
2116+
g_rom[(size_t)kGlyphY * kW + kGlyphX] = 5;
2117+
romFrameForUpscale = g_rom.data();
2118+
Colorize_Framev2(g_rom.data(), kFrameId);
2119+
}
2120+
2121+
// One rotation step on both planes, however long ago the last one was.
2122+
static void StepGlyphRotation(void) {
2123+
colorshiftinittime32[0] = 0;
2124+
colorshiftinittime64[0] = 0;
2125+
Serum_ApplyRotationsv2();
2126+
}
2127+
2128+
static void Test_DynamicContentAndItsShadowDoNotRotate32(void) {
2129+
RenderGlyphOverRotatingBackground(kRequest32);
2130+
CHECK(mySerum.flags & FLAG_RETURNED_32P_FRAME_OK);
2131+
const size_t glyph = (size_t)kGlyphY * kW + kGlyphX;
2132+
const size_t above = glyph - kW, below = glyph + kW;
2133+
const size_t star = (size_t)kStarY * kW + kStarX;
2134+
CHECK_EQ(mySerum.frame32[glyph], kStarB);
2135+
CHECK_EQ(mySerum.frame32[above], kGlyphShadow);
2136+
CHECK_EQ(mySerum.frame32[below], kGlyphShadow);
2137+
CHECK_EQ(mySerum.frame32[star], kStarA);
2138+
CHECK_EQ(mySerum.rotationsinframe32[glyph * 2], 0xffff);
2139+
CHECK_EQ(mySerum.rotationsinframe32[above * 2], 0xffff);
2140+
CHECK_EQ(mySerum.rotationsinframe32[above * 2 + 1], 0xffff);
2141+
CHECK_EQ(mySerum.rotationsinframe32[below * 2], 0xffff);
2142+
CHECK_EQ(mySerum.rotationsinframe32[below * 2 + 1], 0xffff);
2143+
CHECK_EQ(mySerum.rotationsinframe32[star * 2], 0);
2144+
// Every pixel is either the background, in the rotation, or not in one.
2145+
const size_t px = (size_t)kW * kCH;
2146+
unsigned rotating = 0;
2147+
for (size_t i = 0; i < px; ++i)
2148+
if (mySerum.rotationsinframe32[i * 2] != 0xffff) ++rotating;
2149+
CHECK_EQ(rotating, px - 3);
2150+
2151+
for (int step = 1; step <= 4; ++step) {
2152+
StepGlyphRotation();
2153+
CHECK_EQ(mySerum.frame32[glyph], kStarB);
2154+
CHECK_EQ(mySerum.frame32[above], kGlyphShadow);
2155+
CHECK_EQ(mySerum.frame32[below], kGlyphShadow);
2156+
CHECK_EQ(mySerum.frame32[star], (step & 1) ? kStarB : kStarA);
2157+
}
2158+
TearDownFrame();
2159+
}
2160+
2161+
// The upscaled plane, for a caller that asked for it alone and for one that
2162+
// asked for both: the glyph's 2x2 block and the shadows generated around it
2163+
// stand still while the background behind them rotates.
2164+
static void CheckUpscaledGlyphDoesNotRotate(RotationRequest request) {
2165+
RenderGlyphOverRotatingBackground(request);
2166+
CHECK(mySerum.flags & FLAG_RETURNED_64P_FRAME_OK);
2167+
const uint32_t w = kW * 2;
2168+
const size_t px = (size_t)w * kCH * 2;
2169+
const std::vector<uint16_t> first(mySerum.frame64, mySerum.frame64 + px);
2170+
unsigned glyph = 0, shadow = 0;
2171+
for (size_t i = 0; i < px; ++i) {
2172+
if (first[i] == kStarA) {
2173+
CHECK_EQ(mySerum.rotationsinframe64[i * 2], 0);
2174+
continue;
2175+
}
2176+
// Not the background: the glyph, a shadow, or a frame corner the
2177+
// upscale rounded away.
2178+
if (first[i] == kStarB) ++glyph;
2179+
if (first[i] == kGlyphShadow) ++shadow;
2180+
CHECK_EQ(mySerum.rotationsinframe64[i * 2], 0xffff);
2181+
}
2182+
CHECK_EQ(glyph, 4);
2183+
CHECK(shadow > 0);
2184+
CHECK_EQ(first[(size_t)(kGlyphY * 2) * w + kGlyphX * 2], kStarB);
2185+
2186+
for (int step = 1; step <= 4; ++step) {
2187+
StepGlyphRotation();
2188+
for (size_t i = 0; i < px; ++i) {
2189+
if (first[i] == kStarA)
2190+
CHECK_EQ(mySerum.frame64[i], (step & 1) ? kStarB : kStarA);
2191+
else
2192+
CHECK_EQ(mySerum.frame64[i], first[i]);
2193+
}
2194+
if (request == kRequestBoth) {
2195+
const size_t at = (size_t)kGlyphY * kW + kGlyphX;
2196+
CHECK_EQ(mySerum.frame32[at], kStarB);
2197+
CHECK_EQ(mySerum.frame32[at - kW], kGlyphShadow);
2198+
CHECK_EQ(mySerum.frame32[at + kW], kGlyphShadow);
2199+
CHECK_EQ(mySerum.frame32[(size_t)kStarY * kW + kStarX],
2200+
(step & 1) ? kStarB : kStarA);
2201+
}
2202+
}
2203+
TearDownFrame();
2204+
}
2205+
2206+
static void Test_DynamicContentAndItsShadowDoNotRotate64(void) {
2207+
CheckUpscaledGlyphDoesNotRotate(kRequest64);
2208+
}
2209+
2210+
static void Test_DynamicContentAndItsShadowDoNotRotateBoth(void) {
2211+
CheckUpscaledGlyphDoesNotRotate(kRequestBoth);
2212+
}
2213+
20442214
// ---------------------------------------------------------------------------
20452215
// is256x64 projects
20462216
//
@@ -3837,6 +4007,12 @@ static const TestCase kTests[] = {
38374007
Test_RotationAdvancesTaggedPixelsOnly},
38384008
{"rotation/wraps_at_length", Test_RotationWrapsAtItsLength},
38394009
{"rotation/empty_slot_not_due_next", Test_EmptyRotationSlotIsNotDueNext},
4010+
{"rotation/dynamic_content_and_shadow_32p",
4011+
Test_DynamicContentAndItsShadowDoNotRotate32},
4012+
{"rotation/dynamic_content_and_shadow_64p",
4013+
Test_DynamicContentAndItsShadowDoNotRotate64},
4014+
{"rotation/dynamic_content_and_shadow_both",
4015+
Test_DynamicContentAndItsShadowDoNotRotateBoth},
38404016
{"n256/loads_and_forgets", Test_N256LoadsAndForgets},
38414017
{"n256/rejects_other_geometry", Test_N256RejectsOtherGeometry},
38424018
{"n256/identification", Test_N256Identification},

0 commit comments

Comments
 (0)