fix(spf): apply the origin and pitch rules to the writer and the view

CHANGELOG.md

@@ -33,10 +33,18 @@ documentation, validated against a real 7.41 client install.
   now draws palette index 0 opaque and masks everything outside the isometric diamond to
   transparent. Verified against `TILEA.BMP`: ~417k index-0 pixels across 1,143 tiles and
   1,100 stray padding bytes across 65 tiles were affected.
-- **SPF frames ignored `left`/`top` and `pitch`.** Both the colorized decode in `SpfFile`
-  and the palettized `renderSpfPalettized` path now use `right - left` / `bottom - top`
-  for dimensions and advance rows by the frame's pitch. Affected 190 (offset) + 78 (pitch)
-  of 982 real frames.
+- **SPF frames ignored `left`/`top` and `pitch`.** The colorized decode in `SpfFile`, the
+  colorized decode in `SpfView`, the colorized writer, and the palettized
+  `renderSpfPalettized` path all now use `right - left` / `bottom - top` for dimensions
+  and advance rows by the frame's pitch. Affected 190 (offset) + 78 (pitch) of 982 real
+  frames. Before this, serializing a frame that had just been read threw a `TypeError`,
+  and `SpfView` threw a `RangeError` or returned different pixels than `SpfFile` for the
+  same bytes.
+- **The colorized SPF writer recorded a byte count it did not write.** `startAddress`
+  advanced by the source frame's `byteCount`, which includes row padding, while the
+  payload written is tightly packed. Every frame after the first therefore pointed into
+  the wrong place. The writer now normalizes `byteWidth`, `byteCount` and
+  `imageByteCount` to describe the bytes it emits.
 - **SPF mode bytes were read as one `uint32`.** They are two `u8`s (`pixelMode` at +0x08,
   `paletteMode` at +0x09) followed by two preserved bytes; the embedded palette block is
   gated on both being zero. New `SpfFile.paletteMode` / `reservedModeBytes` fields;

src/drawing/SpfFile.ts

@@ -250,6 +250,16 @@ export class SpfFile {
     let startAddress = 0;
 
     for (const frame of this.frames) {
+      // `colorData` is tightly packed by the reader, so the frame is written with no
+      // row padding. Normalize the pitch and the byte counts to what is actually
+      // emitted — keeping a source file's padded pitch here would make every later
+      // frame's startAddress point into the wrong place.
+      const w = spfFrameWidth(frame);
+      const h = spfFrameHeight(frame);
+      frame.byteWidth = w * 2;
+      frame.imageByteCount = w * h;
+      frame.byteCount = w * h * 4; // the RGB565 copy plus the RGB555 copy
+
       frame.startAddress = startAddress;
       startAddress += frame.byteCount;
       writer.writeUInt16LE(frame.left);
@@ -268,19 +278,19 @@ export class SpfFile {
     writer.writeUInt32LE(startAddress);
 
     for (const frame of this.frames) {
-      const w = frame.right;
-      const h = frame.bottom;
+      // The bounds give the size, exactly as the reader does. Using right/bottom here
+      // would run past the end of colorData for any frame with a non-zero origin.
+      const w = spfFrameWidth(frame);
+      const h = spfFrameHeight(frame);
+      const colorData = frame.colorData!;
+
       // Primary: RGB565
-      for (let y = 0; y < h; y++) {
-        for (let x = 0; x < w; x++) {
-          writer.writeUInt16LE(encodeRgb565(frame.colorData![y * w + x]!));
-        }
+      for (let i = 0; i < w * h; i++) {
+        writer.writeUInt16LE(encodeRgb565(colorData[i] ?? TRANSPARENT));
       }
       // Secondary: RGB555
-      for (let y = 0; y < h; y++) {
-        for (let x = 0; x < w; x++) {
-          writer.writeUInt16LE(encodeRgb555(frame.colorData![y * w + x]!));
-        }
+      for (let i = 0; i < w * h; i++) {
+        writer.writeUInt16LE(encodeRgb555(colorData[i] ?? TRANSPARENT));
       }
     }
   }

src/drawing/virtualized/SpfView.ts

@@ -1,4 +1,6 @@
 import { SpfFormatType } from '../../enums.js';
+import type { Color } from '../../constants.js';
+import { TRANSPARENT } from '../../constants.js';
 import type { DataArchive } from '../../data/DataArchive.js';
 import type { DataArchiveEntry } from '../../data/DataArchiveEntry.js';
 import { SpanReader } from '../../io/SpanReader.js';
@@ -64,13 +66,24 @@ export class SpfView {
     if (this.format === SpfFormatType.Palettized) {
       frame.data = new Uint8Array(buf.subarray(dataStart, dataStart + t.byteCount));
     } else {
-      // Colorized: read RGB565 color words
-      const colorData = [];
-      const reader = new SpanReader(buf);
-      reader.seek(dataStart);
-      const pixelCount = t.bottom * t.right;
-      for (let i = 0; i < pixelCount; i++) {
-        colorData.push(decodeRgb565(reader.readUInt16LE()));
+      // Colorized: RGB565 color words. This must match SpfFile.readColorized exactly —
+      // the bounds give the visible rectangle, and rows advance by the frame's own
+      // pitch, which is not always the row's byte width.
+      const w = t.right - t.left;
+      const h = t.bottom - t.top;
+      const stride = t.byteWidth > 0 ? t.byteWidth : w * 2;
+      const pixels = buf.subarray(dataStart, dataStart + t.byteCount);
+
+      const colorData = new Array<Color>(w * h);
+      for (let y = 0; y < h; y++) {
+        const rowOffset = y * stride;
+        for (let x = 0; x < w; x++) {
+          const at = rowOffset + x * 2;
+          colorData[y * w + x] =
+            at + 1 < pixels.length
+              ? decodeRgb565(pixels[at]! | (pixels[at + 1]! << 8))
+              : TRANSPARENT;
+        }
       }
       frame.colorData = colorData;
     }

tests/spfFile.test.ts

@@ -291,6 +291,141 @@ describe('SpfFile', () => {
     });
   });
 
+  // Real colorized art is not anchored at the origin and is not always tightly
+  // packed: of 982 frames in the client, 190 have a non-zero left/top and 78 have a
+  // pitch that differs from the row width. Every fixture built by
+  // `fromColorizedRgbaFrames` has left = top = 0 and byteWidth = width * 2, which is
+  // exactly the case where a bounds bug and an origin bug cancel out — so these
+  // fixtures are hand-built to keep the two apart.
+  describe('offset and padded colorized frames', () => {
+    interface OffsetSpec {
+      left: number; top: number; right: number; bottom: number;
+      /** Extra bytes per row beyond width * 2. */
+      padding?: number;
+    }
+
+    /** Build a one-frame colorized SPF whose rows carry `padding` trailing bytes. */
+    function offsetColorizedSpf({ left, top, right, bottom, padding = 0 }: OffsetSpec): Uint8Array {
+      const w = right - left;
+      const h = bottom - top;
+      const stride = w * 2 + padding;
+      const byteCount = stride * h;
+
+      const writer = new SpanWriter();
+      writer.writeUInt32LE(0);
+      writer.writeUInt32LE(0);
+      writer.writeUInt8(2); // format = Colorized
+      writer.writeUInt8(0);
+      writer.writeUInt8(0);
+      writer.writeUInt8(0);
+      writer.writeUInt32LE(1); // frameCount
+
+      writer.writeUInt16LE(left);
+      writer.writeUInt16LE(top);
+      writer.writeUInt16LE(right);
+      writer.writeUInt16LE(bottom);
+      writer.writeInt16LE(0);
+      writer.writeInt16LE(0);
+      writer.writeUInt32LE(0);
+      writer.writeUInt32LE(0);          // startAddress
+      writer.writeUInt32LE(stride);     // byteWidth — the pitch
+      writer.writeUInt32LE(byteCount);
+      writer.writeUInt32LE(w * h);
+      writer.writeUInt32LE(byteCount);
+
+      // Each row is a distinct color, then `padding` bytes of filler. If a reader
+      // ignores the pitch it lands in the filler and decodes the wrong row.
+      const rowColors = [
+        { r: 255, g: 0, b: 0, a: 255 },
+        { r: 0, g: 255, b: 0, a: 255 },
+        { r: 0, g: 0, b: 255, a: 255 },
+        { r: 255, g: 255, b: 0, a: 255 },
+      ];
+      for (let y = 0; y < h; y++) {
+        const color = rowColors[y % rowColors.length]!;
+        for (let x = 0; x < w; x++) writer.writeUInt16LE(encodeRgb565(color));
+        for (let p = 0; p < padding; p++) writer.writeUInt8(0);
+      }
+
+      return writer.toUint8Array();
+    }
+
+    const OFFSET: OffsetSpec = { left: 2, top: 1, right: 6, bottom: 4, padding: 4 };
+
+    it('sizes colorData from the bounds, not from right and bottom', () => {
+      const frame = SpfFile.fromBuffer(offsetColorizedSpf(OFFSET)).frames[0]!;
+      expect(frame.colorData).toHaveLength((6 - 2) * (4 - 1));
+    });
+
+    it('reads each row at its own pitch', () => {
+      const frame = SpfFile.fromBuffer(offsetColorizedSpf(OFFSET)).frames[0]!;
+      const w = 6 - 2;
+      // Row 0 red, row 1 green, row 2 blue. Ignoring the pitch would put row 1 inside
+      // row 0's padding and decode it as black.
+      expect(frame.colorData![0]!.r).toBeGreaterThan(240);
+      expect(frame.colorData![w]!.g).toBeGreaterThan(240);
+      expect(frame.colorData![2 * w]!.b).toBeGreaterThan(240);
+    });
+
+    // Regression: the writer used right/bottom as the size while the reader used the
+    // bounds, so serializing a frame the reader had just produced ran off the end of
+    // colorData and threw.
+    it('serializes a frame it just read, without running off the end', () => {
+      const spf = SpfFile.fromBuffer(offsetColorizedSpf(OFFSET));
+      expect(() => spf.toUint8Array()).not.toThrow();
+    });
+
+    it('round-trips an offset padded frame back to the same pixels', () => {
+      const original = SpfFile.fromBuffer(offsetColorizedSpf(OFFSET));
+      const reparsed = SpfFile.fromBuffer(original.toUint8Array());
+      const before = original.frames[0]!;
+      const after = reparsed.frames[0]!;
+
+      expect(after.left).toBe(2);
+      expect(after.top).toBe(1);
+      expect(after.colorData).toHaveLength(before.colorData!.length);
+      expect(after.colorData).toEqual(before.colorData);
+    });
+
+    // The writer emits tightly packed rows, so the pitch and the byte counts it
+    // records must describe what it actually wrote. If they still described the
+    // source file's padded rows, the next frame's startAddress would be wrong.
+    it('normalizes the pitch and byte counts to the bytes it writes', () => {
+      const spf = SpfFile.fromBuffer(offsetColorizedSpf(OFFSET));
+      spf.toUint8Array();
+      const frame = spf.frames[0]!;
+      const w = 6 - 2;
+      const h = 4 - 1;
+
+      expect(frame.byteWidth).toBe(w * 2);
+      expect(frame.imageByteCount).toBe(w * h);
+      expect(frame.byteCount).toBe(w * h * 4);
+    });
+
+    it('keeps every frame addressable when several are written', () => {
+      const spf = SpfFile.fromBuffer(offsetColorizedSpf(OFFSET));
+      // A second frame with different bounds, sharing the first frame's pixels.
+      spf.frames.push({
+        ...spf.frames[0]!,
+        left: 0, top: 0, right: 4, bottom: 3,
+        colorData: spf.frames[0]!.colorData,
+      });
+
+      const reparsed = SpfFile.fromBuffer(spf.toUint8Array());
+      expect(reparsed.frames).toHaveLength(2);
+      // Frame 1 starts exactly where frame 0 ends.
+      expect(reparsed.frames[1]!.startAddress).toBe(reparsed.frames[0]!.byteCount);
+      expect(reparsed.frames[1]!.colorData![0]!.r).toBeGreaterThan(240);
+    });
+
+    it('pads with transparent when a row is truncated', () => {
+      const frame = SpfFile.fromBuffer(
+        offsetColorizedSpf({ left: 0, top: 0, right: 4, bottom: 2, padding: 0 }),
+      ).frames[0]!;
+      expect(frame.colorData).toHaveLength(8);
+    });
+  });
+
   describe('mode bytes', () => {
     it('rejects an unknown pixel mode', () => {
       const writer = new SpanWriter();

tests/virtualizedViews.test.ts

@@ -1,6 +1,8 @@
 import { describe, expect, it } from 'vitest';
 import { buildArchive } from './archiveFixture.js';
 import { TILE_SIZE } from '../src/constants.js';
+import { SpanWriter } from '../src/io/SpanWriter.js';
+import { encodeRgb565 } from '../src/utility/ColorCodec.js';
 import { EpfFile } from '../src/drawing/EpfFile.js';
 import { SpfFile } from '../src/drawing/SpfFile.js';
 import { SpfFormatType } from '../src/enums.js';
@@ -163,5 +165,79 @@ describe('SpfView', () => {
       expect(view.tryGet(1)).toBeUndefined();
       expect(view.tryGet(0)).toBeDefined();
     });
+
+    /**
+     * The parity test above cannot catch an origin or pitch bug on its own: every
+     * frame `fromColorizedRgbaFrames` builds has left = top = 0 and
+     * byteWidth = width * 2, which is exactly where the correct and the incorrect
+     * conventions agree. This fixture is hand-built so they diverge.
+     */
+    describe('with a non-zero origin and a padded pitch', () => {
+      function offsetColorizedSpf(): Uint8Array {
+        const left = 2, top = 1, right = 6, bottom = 4, padding = 4;
+        const w = right - left, h = bottom - top;
+        const stride = w * 2 + padding;
+        const byteCount = stride * h;
+
+        const wr = new SpanWriter();
+        wr.writeUInt32LE(0);
+        wr.writeUInt32LE(0);
+        wr.writeUInt8(2); // format = Colorized
+        wr.writeUInt8(0);
+        wr.writeUInt8(0);
+        wr.writeUInt8(0);
+        wr.writeUInt32LE(1); // frameCount
+
+        wr.writeUInt16LE(left);
+        wr.writeUInt16LE(top);
+        wr.writeUInt16LE(right);
+        wr.writeUInt16LE(bottom);
+        wr.writeInt16LE(0);
+        wr.writeInt16LE(0);
+        wr.writeUInt32LE(0);
+        wr.writeUInt32LE(0);
+        wr.writeUInt32LE(stride);
+        wr.writeUInt32LE(byteCount);
+        wr.writeUInt32LE(w * h);
+        wr.writeUInt32LE(byteCount);
+
+        const rowColors = [
+          { r: 255, g: 0, b: 0, a: 255 },
+          { r: 0, g: 255, b: 0, a: 255 },
+          { r: 0, g: 0, b: 255, a: 255 },
+        ];
+        for (let y = 0; y < h; y++) {
+          const color = rowColors[y % rowColors.length]!;
+          for (let x = 0; x < w; x++) wr.writeUInt16LE(encodeRgb565(color));
+          for (let p = 0; p < padding; p++) wr.writeUInt8(0);
+        }
+
+        return wr.toUint8Array();
+      }
+
+      it('reads the same pixels as the eager parser', () => {
+        const bytes = offsetColorizedSpf();
+        const view = SpfView.fromArchive('fx', buildArchive([{ name: 'fx.spf', data: bytes }]));
+        const eager = SpfFile.fromBuffer(bytes);
+
+        const lazy = view.get(0);
+        expect(lazy.colorData).toHaveLength(eager.frames[0]!.colorData!.length);
+        expect(lazy.colorData).toEqual(eager.frames[0]!.colorData);
+      });
+
+      it('sizes the frame from the bounds and honours the pitch', () => {
+        const view = SpfView.fromArchive(
+          'fx',
+          buildArchive([{ name: 'fx.spf', data: offsetColorizedSpf() }]),
+        );
+        const frame = view.get(0);
+        const w = 6 - 2;
+
+        expect(frame.colorData).toHaveLength(w * (4 - 1));
+        expect(frame.colorData![0]!.r).toBeGreaterThan(240);
+        expect(frame.colorData![w]!.g).toBeGreaterThan(240);
+        expect(frame.colorData![2 * w]!.b).toBeGreaterThan(240);
+      });
+    });
   });
 });