[p5.js 2.0+ Bug Report]: default color serialization truncates channel bytes, disagreeing with #rrggbb on the same color

Open Beginner friendly
#9,124 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
78/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
javascript
Domain
computer-graphics

Research direction

Start with toHexComponent in src/color/p5.Color.js, then inspect the fillStyle and strokeStyle call sites in src/core/p5.Renderer2D.js. Reproduce the fractional-channel examples and run the color test suite, including the three proposed regression cases. Done means default serialization and #rrggbb serialization agree while the existing color tests continue to pass.

Written by the indexing model from the issue text.

Description

Most appropriate sub-area of p5.js?

Color

p5.js version

2.x main (4b096e2)

Actual vs expected behavior

toHexComponent in src/color/p5.Color.js truncates instead of rounding:

const vInt = ~~(v * 255);

The cached default string (returned by toString() and fed by p5.Renderer2D into canvas fillStyle/strokeStyle at src/core/p5.Renderer2D.js:187,218,236) therefore disagrees with toString('#rrggbb') on the very same color object, because the #rrggbb path goes through colorjs and rounds:

colorMode(RGB, 100);
const c = color(50, 0, 0);
c.toString();          // '#7f0000'  (truncated)
c.toString('#rrggbb'); // '#800000'  (rounded)

color(127.5, 0, 0).toString();       // '#7f0000', expected '#800000'
color(255, 0, 102, 127.5).toString(); // alpha 7f, expected 80

So any fractional channel byte renders one step darker than the canonical serialization of the same object, and the two serializations of one color cannot both be right.

Steps to reproduce

Run the snippet above in any 2.x sketch, or execute new p5.Color(...) paths directly under node; outputs above are from executed runs against current main.

Note

I have a one-line fix (round to nearest byte) with unit tests ready, verified that the three new tests fail on current main and pass with the change while the other 92 color tests pass either way. Per the contributing guide I am filing this for approval before opening the PR. Happy to submit it as soon as this is approved for implementation.

Dominant language
JavaScript
Stars
24k
Forks
3.8k
Avg merge
3d 9h
Merged PRs (30d)
26

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from processing/p5.js

All issues in processing/p5.js

Similar issues

More JavaScript issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.