Skip to content

Fixes #9000 #9014

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

Open
NalinDalal wants to merge 8 commits into
base: main
Choose a base branch
from
+18 −9
Open

Fixes #9000 #9014

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
88e1550
perf: cache _getRGBA and fast-path RGB mode in p5.Color
NalinDalal Jul 24, 2026
9c6416e
Merge branch 'processing:main' into main
NalinDalal Jul 24, 2026
76da9c5
Merge branch 'processing:main' into main
NalinDalal Jul 27, 2026
7f4b60a
fix: cache only srgb-converted coords
NalinDalal Jul 27, 2026
f60fccb
Merge branch 'main' into main
NalinDalal Jul 30, 2026
2a15b2a
rm test
NalinDalal Jul 30, 2026
c15b88a
Implementing the key-based memoization approach using color coordinat…
NalinDalal Jul 30, 2026
e44102c
Merge branch 'processing:main' into main
NalinDalal Aug 6, 2026
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
31 changes: 21 additions & 10 deletions src/color/p5.Color.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,11 @@ class Color {
// where if we `import { Color }` directly, it will be a separate copy of the
// Color class from the one imported in the main p5.js bundle.
isColor = true;
#srgbCoords = null;

#invalidateSrgbCache() {
this.#srgbCoords = null;
}

// Used to add additional color modes to p5.js
// Uses underlying library's definition
Expand Down Expand Up @@ -241,6 +246,7 @@ class Color {
this._initialize();
this._initialize = undefined;
}
this.#invalidateSrgbCache();
this._cachedColor = newColor;
}

Expand Down Expand Up @@ -559,6 +565,7 @@ class Color {

if(this.mode === RGB || this.mode === RGBP3){
this._color.coords[0] = newval;
this.#invalidateSrgbCache();
}else{
// Will do an imprecise conversion to 'srgb', not recommended
const space = this._color.space.id;
Expand Down Expand Up @@ -611,6 +618,7 @@ class Color {

if(this.mode === RGB || this.mode === RGBP3){
this._color.coords[1] = newval;
this.#invalidateSrgbCache();
}else{
// Will do an imprecise conversion to 'srgb', not recommended
const space = this._color.space.id;
Expand Down Expand Up @@ -663,6 +671,7 @@ class Color {

if(this.mode === RGB || this.mode === RGBP3){
this._color.coords[2] = newval;
this.#invalidateSrgbCache();
}else{
// Will do an imprecise conversion to 'srgb', not recommended
const space = this._color.space.id;
Expand Down Expand Up @@ -715,33 +724,35 @@ class Color {
const newval = map(new_alpha, max[0], max[1], colorjsMax[0], colorjsMax[1]);

this._color.alpha = newval;
this.#invalidateSrgbCache();
}

_getRGBA(maxes=[1, 1, 1, 1]) {
// Get colorjs maxes
const colorjsMaxes = Color.#colorjsMaxes[RGB];

// Normalize everything to 0,1 or the provided range (map)
let coords = structuredClone(to(this._color, 'srgb').coords);
coords.push(this._color.alpha);
// Cache the srgb conversion (the expensive step)
if (!this.#srgbCoords) {
if (this.mode === RGB || this._color.space.id === 'srgb') {
this.#srgbCoords = [...this._color.coords, this._color.alpha];
} else {
this.#srgbCoords = [...to(this._color, 'srgb').coords, this._color.alpha];
}
Comment on lines +732 to +734

@limzykenneth limzykenneth Jul 30, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Good to have this. I wonder if 1000 is the right number? What would a typical number of colors set() would manipulate and what would be a sensible upper bound without it leaking memory?

@NalinDalal NalinDalal Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Typical sketches use far fewer than 1000 unique colors. However, image processing or complex generative art could exceed this. Each entry is small (~32 bytes), so 1000 entries ≈ 32KB max. I chose 1000 as a conservative default that covers most cases without significant memory impact. Would you prefer a different value or should we make it configurable?

@limzykenneth limzykenneth Aug 1, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We can keep it as 1000 for now if the above benchmark still holds.

}

const rangeMaxes = maxes.map((v) => {
const colorjsMaxes = Color.#colorjsMaxes[RGB];
const rangeMaxes = maxes.map(v => {
if(!Array.isArray(v)){
return [0, v];
}else{
return v;
}
});

coords = coords.map((coord, i) => {
return this.#srgbCoords.map((coord, i) => {
return map(
coord,
colorjsMaxes[i][0], colorjsMaxes[i][1],
rangeMaxes[i][0], rangeMaxes[i][1]
);
});

return coords;
}

_getMode() {
Expand Down
71 changes: 71 additions & 0 deletions test/bench/set_benchmark.html
View file
Open in desktop

@limzykenneth limzykenneth Jul 29, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This file shouldn't be committed as it does not match the required format of the benchmark harness.

Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8">
<title>p5.Graphics.set() benchmark</title>
<script src="../../lib/p5.js"></script>
<style>
body { font-family: monospace; padding: 20px; background: #1a1a1a; color: #eee; }
#output { white-space: pre-wrap; line-height: 1.6; }
.highlight { color: #7fdbca; }
.warn { color: #ffcc66; }
</style>
</head>
<body>
<h2>p5.Graphics.set() benchmark</h2>
<div id="output">Starting benchmark...</div>

<script>
const output = document.getElementById('output');
const W = 100, H = 100;
const ITERATIONS = 50; // frames to average
const FPS_OVERHEAD_MS = 1; // rough p5 overhead per frame

let myp5;
let results = [];

function log(msg, cls = '') {
output.innerHTML += (cls ? `<span class="${cls}">${msg}</span>` : msg) + '\n';
}

function run() {
myp5 = new p5(function (p) {
p.setup = function () {
p.noCanvas();
const buf = p.createGraphics(W, H);
const col = p.color(255, 0, 0);

log(`Buffer: ${W}x${H}, iterations: ${ITERATIONS}`);
log('Running set() benchmark...\n');

for (let i = 0; i < ITERATIONS; i++) {
const t0 = performance.now();
for (let y = 0; y < H; y++) {
for (let x = 0; x < W; x++) {
buf.set(x, y, col);
}
}
const t1 = performance.now();
results.push(t1 - t0);
}

const avg = results.reduce((a, b) => a + b, 0) / results.length;
const min = Math.min(...results);
const max = Math.max(...results);

log(`Results (${ITERATIONS} iterations):`);
log(` Average: ${avg.toFixed(2)} ms`);
log(` Min: ${min.toFixed(2)} ms`);
log(` Max: ${max.toFixed(2)} ms`);
log(`\np5.js version: ${p5.version || 'unknown'}`);

myp5.remove();
};
});
}

// small delay so p5 loads
setTimeout(run, 100);
</script>
</body>
</html>
Loading