X Tutup
Skip to content
/ scummvm Public

IMAGE: Replace palette byte arrays with Palette class #6447

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.

Sign up for GitHub

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

Merged
bluegr merged 2 commits into from
Feb 26, 2025
Merged

IMAGE: Replace palette byte arrays with Palette class #6447

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
531b365
IMAGE: Replace palette byte arrays with Palette class
OMGPizzaGuy Feb 21, 2025
b108495
HOPKINS: Use constexpr for conflicting constants
OMGPizzaGuy Feb 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions engines/hopkins/graphics.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,10 @@

namespace Hopkins {

#define DIRTY_RECTS_SIZE 250
#define PALETTE_SIZE 256
#define PALETTE_BLOCK_SIZE (PALETTE_SIZE * 3)
#define PALETTE_EXT_BLOCK_SIZE 800
constexpr int DIRTY_RECTS_SIZE = 250;
constexpr int PALETTE_SIZE = 256;
constexpr int PALETTE_BLOCK_SIZE = (PALETTE_SIZE * 3);
constexpr int PALETTE_EXT_BLOCK_SIZE = 800;
static const byte kSetOffset = 251;
static const byte kByteStop = 252;
static const byte k8bVal = 253;
Expand Down
26 changes: 21 additions & 5 deletions graphics/palette.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,27 @@ bool Palette::contains(const Palette& p) const {
return p._size <= _size && !memcmp(_data, p._data, p._size * 3);
}

void Palette::clear() {
delete[] _data;
_data = nullptr;
_size = 0;
}

void Palette::resize(uint newSize, bool preserve) {
if (newSize > _size) {
byte *newData = nullptr;
if (newSize > 0) {
newData = new byte[newSize * 3]();
Copy link
Member

@lephilousophe lephilousophe Feb 22, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may be interesting to switch to malloc/free and use realloc here.
And maybe adding a flag to ask for old palette preservation: that would avoid a useless copy.

Copy link
Member Author

@OMGPizzaGuy OMGPizzaGuy Feb 25, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Realloc would handle a copy for us. We could look into switching, but it feels a bit outside the scope of this PR.
I'm not worried about optimizing out an extra copy though; resizing a palette should be a rare occurrence, especially from anything other than of than empty palette.

Copy link
Member

@lephilousophe lephilousophe Feb 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So adding a flag to ask for preserving the contents could avoid a useless copy

Copy link
Member Author

@OMGPizzaGuy OMGPizzaGuy Feb 25, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, preserve flag is added.

if (_size > 0 && preserve)
memcpy(newData, _data, _size * 3);
}

delete[] _data;
_data = newData;
}
_size = newSize;
}

byte Palette::findBestColor(byte cr, byte cg, byte cb, ColorDistanceMethod method) const {
uint bestColor = 0;
uint32 min = 0xFFFFFFFF;
Expand Down Expand Up @@ -127,11 +148,6 @@ byte Palette::findBestColor(byte cr, byte cg, byte cb, ColorDistanceMethod metho
return bestColor;
}

void Palette::clear() {
if (_size > 0)
memset(_data, 0, _size);
}

void Palette::set(const byte *colors, uint start, uint num) {
assert(start < _size && (start + num) <= _size);
memcpy(_data + 3 * start, colors, 3 * num);
Expand Down
15 changes: 13 additions & 2 deletions graphics/palette.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,19 @@ class Palette {

const byte *data() const { return _data; }
uint size() const { return _size; }

/**
* Clears the palette of all entries and resets the size to zero.
*/
void clear();

/**
* Changes the number of palette entries.
*
* @param newSize the new number of palette entries
* @param preserve indicates the existing entry values should be preserved
*/
void resize(uint newSize, bool preserve);

void set(uint entry, byte r, byte g, byte b) {
assert(entry < _size);
Expand Down Expand Up @@ -125,8 +138,6 @@ class Palette {
*/
byte findBestColor(byte r, byte g, byte b, ColorDistanceMethod method = kColorDistanceRedmean) const;

void clear();

/**
* Replace the specified range of the palette with new colors.
* The palette entries from 'start' till (start+num-1) will be replaced - so
Expand Down
36 changes: 15 additions & 21 deletions image/bmp.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,27 +36,19 @@

namespace Image {

BitmapDecoder::BitmapDecoder() {
_surface = 0;
_palette = 0;
_paletteColorCount = 0;
_codec = 0;
BitmapDecoder::BitmapDecoder(): _codec(nullptr), _surface(nullptr), _palette(0) {
}

BitmapDecoder::~BitmapDecoder() {
destroy();
}

void BitmapDecoder::destroy() {
_surface = 0;

delete[] _palette;
_palette = 0;

_paletteColorCount = 0;

delete _codec;
_codec = 0;
_codec = nullptr;

_surface = nullptr;
_palette.clear();
}

bool BitmapDecoder::loadStream(Common::SeekableReadStream &stream) {
Expand Down Expand Up @@ -111,22 +103,24 @@ bool BitmapDecoder::loadStream(Common::SeekableReadStream &stream) {
uint32 imageSize = stream.readUint32LE();
/* uint32 pixelsPerMeterX = */ stream.readUint32LE();
/* uint32 pixelsPerMeterY = */ stream.readUint32LE();
_paletteColorCount = stream.readUint32LE();
uint32 paletteColorCount = stream.readUint32LE();
/* uint32 colorsImportant = */ stream.readUint32LE();

stream.seek(infoSize - 40, SEEK_CUR);

if (bitsPerPixel == 4 || bitsPerPixel == 8) {
if (_paletteColorCount == 0)
_paletteColorCount = bitsPerPixel == 8 ? 256 : 16;
if (paletteColorCount == 0)
paletteColorCount = bitsPerPixel == 8 ? 256 : 16;

// Read the palette
_palette = new byte[_paletteColorCount * 3];
for (uint16 i = 0; i < _paletteColorCount; i++) {
_palette[i * 3 + 2] = stream.readByte();
_palette[i * 3 + 1] = stream.readByte();
_palette[i * 3 + 0] = stream.readByte();
_palette.resize(paletteColorCount, false);
for (uint16 i = 0; i < paletteColorCount; i++) {
byte b = stream.readByte();
byte g = stream.readByte();
byte r = stream.readByte();
stream.readByte();

_palette.set(i, r, g, b);
}
}

Expand Down
8 changes: 4 additions & 4 deletions image/bmp.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@

#include "common/scummsys.h"
#include "common/str.h"
#include "graphics/palette.h"
#include "image/image_decoder.h"

namespace Common {
Expand Down Expand Up @@ -74,14 +75,13 @@ class BitmapDecoder : public ImageDecoder {
void destroy();
virtual bool loadStream(Common::SeekableReadStream &stream);
virtual const Graphics::Surface *getSurface() const { return _surface; }
const byte *getPalette() const { return _palette; }
uint16 getPaletteColorCount() const { return _paletteColorCount; }
const byte *getPalette() const { return _palette.data(); }
uint16 getPaletteColorCount() const { return _palette.size(); }

private:
Codec *_codec;
const Graphics::Surface *_surface;
byte *_palette;
uint16 _paletteColorCount;
Graphics::Palette _palette;
};

/**
Expand Down
13 changes: 3 additions & 10 deletions image/cel_3do.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,23 +36,16 @@ enum CCBFlags {
kCCBNoPre0 = 1 << 22
};

Cel3DODecoder::Cel3DODecoder() {
_surface = 0;
_palette = 0;
_paletteColorCount = 0;
Cel3DODecoder::Cel3DODecoder(): _surface(nullptr), _palette(0) {
}

Cel3DODecoder::~Cel3DODecoder() {
destroy();
}

void Cel3DODecoder::destroy() {
_surface = 0;

delete[] _palette;
_palette = 0;

_paletteColorCount = 0;
_surface = nullptr;
_palette.clear();
}

bool Cel3DODecoder::loadStream(Common::SeekableReadStream &stream) {
Expand Down
8 changes: 4 additions & 4 deletions image/cel_3do.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

#include "common/scummsys.h"
#include "common/str.h"
#include "graphics/palette.h"
#include "image/image_decoder.h"

namespace Common {
Expand Down Expand Up @@ -56,13 +57,12 @@ class Cel3DODecoder : public ImageDecoder {
void destroy();
virtual bool loadStream(Common::SeekableReadStream &stream);
virtual const Graphics::Surface *getSurface() const { return _surface; }
const byte *getPalette() const { return _palette; }
uint16 getPaletteColorCount() const { return _paletteColorCount; }
const byte *getPalette() const { return _palette.data(); }
uint16 getPaletteColorCount() const { return _palette.size(); }

private:
const Graphics::Surface *_surface;
byte *_palette;
uint16 _paletteColorCount;
Graphics::Palette _palette;
};
/** @} */
} // End of namespace Image
Expand Down
28 changes: 10 additions & 18 deletions image/cicn.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,7 @@

namespace Image {

CicnDecoder::CicnDecoder() {
_surface = nullptr;
_palette = nullptr;
_paletteColorCount = 0;
_mask = nullptr;
CicnDecoder::CicnDecoder(): _surface(nullptr), _palette(0), _mask(nullptr) {
}

CicnDecoder::~CicnDecoder() {
Expand All @@ -46,11 +42,8 @@ void CicnDecoder::destroy() {
delete _surface;
_surface = nullptr;
}

delete[] _palette;
_palette = nullptr;
_paletteColorCount = 0;


_palette.clear();
if (_mask) {
_mask->free();
delete _mask;
Expand Down Expand Up @@ -106,17 +99,16 @@ bool CicnDecoder::loadStream(Common::SeekableReadStream &stream) {

// Palette
stream.skip(6);
_paletteColorCount = stream.readUint16BE() + 1;

_palette = new byte[3 * _paletteColorCount];
uint16 paletteColorCount = stream.readUint16BE() + 1;

byte *p = _palette;
_palette.resize(paletteColorCount, false);

for (uint i = 0; i < _paletteColorCount; i++) {
for (uint i = 0; i < paletteColorCount; i++) {
stream.skip(2);
*p++ = stream.readUint16BE() >> 8;
*p++ = stream.readUint16BE() >> 8;
*p++ = stream.readUint16BE() >> 8;
byte r = stream.readUint16BE() >> 8;
byte g = stream.readUint16BE() >> 8;
byte b = stream.readUint16BE() >> 8;
_palette.set(i, r, g, b);
}

_surface = new Graphics::Surface();
Expand Down
8 changes: 4 additions & 4 deletions image/cicn.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#ifndef IMAGE_CICN_H
#define IMAGE_CICN_H

#include "graphics/palette.h"
#include "image/image_decoder.h"

namespace Image {
Expand All @@ -46,14 +47,13 @@ class CicnDecoder : public ImageDecoder {
void destroy() override;
bool loadStream(Common::SeekableReadStream &stream) override;
const Graphics::Surface *getSurface() const override { return _surface; }
const byte *getPalette() const override { return _palette; }
uint16 getPaletteColorCount() const override { return _paletteColorCount; }
const byte *getPalette() const override { return _palette.data(); }
uint16 getPaletteColorCount() const override { return _palette.size(); }
const Graphics::Surface *getMask() const override { return _mask; }

private:
Graphics::Surface *_surface;
byte *_palette;
uint16 _paletteColorCount;
Graphics::Palette _palette;
Graphics::Surface *_mask;
};

Expand Down
19 changes: 8 additions & 11 deletions image/gif.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@

namespace Image {

GIFDecoder::GIFDecoder() : _outputSurface(0), _palette(0), _colorCount(0) {
GIFDecoder::GIFDecoder() : _outputSurface(nullptr), _palette(0) {
}

GIFDecoder::~GIFDecoder() {
Expand Down Expand Up @@ -89,15 +89,15 @@ bool GIFDecoder::loadStream(Common::SeekableReadStream &stream) {
}
}

_colorCount = colorMap->ColorCount;
int colorCount = colorMap->ColorCount;
_outputSurface = new Graphics::Surface();
_palette = new uint8[_colorCount * 3];
_palette.resize(colorCount, false);

const Graphics::PixelFormat format = Graphics::PixelFormat::createFormatCLUT8();
for (int i = 0; i < _colorCount; ++i) {
_palette[(i * 3) + 0] = colorMap->Colors[i].Red;
_palette[(i * 3) + 1] = colorMap->Colors[i].Green;
_palette[(i * 3) + 2] = colorMap->Colors[i].Blue;
for (int i = 0; i < colorCount; ++i) {
_palette.set(i, colorMap->Colors[i].Red,
colorMap->Colors[i].Green,
colorMap->Colors[i].Blue);
}

// TODO: support transparency
Expand Down Expand Up @@ -128,10 +128,7 @@ void GIFDecoder::destroy() {
delete _outputSurface;
_outputSurface = 0;
}
if (_palette) {
delete[] _palette;
_palette = 0;
}
_palette.clear();
}

} // End of namespace Image
8 changes: 4 additions & 4 deletions image/gif.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#ifndef IMAGE_GIF_H
#define IMAGE_GIF_H

#include "graphics/palette.h"
#include "image/image_decoder.h"

#ifdef USE_GIF
Expand Down Expand Up @@ -55,15 +56,14 @@ class GIFDecoder : public ImageDecoder {

bool loadStream(Common::SeekableReadStream &stream) override;
void destroy() override;
const byte *getPalette() const override { return _palette; }
uint16 getPaletteColorCount() const override { return _colorCount; }
const byte *getPalette() const override { return _palette.data(); }
uint16 getPaletteColorCount() const override { return _palette.size(); }
const Graphics::Surface *getSurface() const override { return _outputSurface; }
bool hasTransparentColor() const override { return _hasTransparentColor; }
uint32 getTransparentColor() const override { return _transparentColor; }
private:
Graphics::Surface *_outputSurface;
uint8 *_palette;
uint16 _colorCount;
Graphics::Palette _palette;
bool _hasTransparentColor;
uint32 _transparentColor;
};
Expand Down
Loading
Loading
X Tutup