Skip to content

Conversation

@Charlie-83
Copy link
Contributor

@Charlie-83 Charlie-83 commented Nov 11, 2023

I agree that my contributions are licensed under the Individual Contributor License Agreement V4.0 ("CLA") as stated in https://github.com/igarastudio/cla/blob/main/cla.md

I have signed the CLA following the steps given in https://github.com/igarastudio/cla#signing

This PR aims to fix numerical issues with the gradient tool. To reproduce the issue, do the following:

  1. Create a new sprite with width 57 and height 1.
  2. Select #00e000 as primary colour and #000000 as secondary.
  3. Select the gradient tool and drag starting from (57, 0) and ending at (0, 0)

Expectation: the green value of each pixel is equal to four times its x value.
Reality: some pixels have a green value equal to 4x-1

This is caused by numerical errors giving a green value of, say, 63.9999999999999 which, when cast to int, becomes 63.

I have done two things to try to fix this and applied my changes to both the linear and radial gradients.

  1. Remove the unnecessary dividing, then later multiplying, by 255 and keep the colour values as ints.
  2. Add a small value (1e7) to the calculated values to avoid errors caused by the division in double f = (q * w) / wmag

While I'm sure that almost no one else cares about such small errors in the produced colours, they have been very annoying for me as I have been using aseprite to generate position maps (images where RGB correspond to XYZ coordinates in 3D space). The gradient tool should be very useful for generating sequences of values but isn't due to this issue.

@Charlie-83 Charlie-83 requested a review from dacap as a code owner November 11, 2023 11:22
@Charlie-83 Charlie-83 marked this pull request as draft November 11, 2023 11:23
@Charlie-83

This comment was marked as resolved.

@Charlie-83 Charlie-83 marked this pull request as ready for review November 11, 2023 11:31
@Charlie-83
Copy link
Contributor Author

Would anyone be able to review this at some point?

@Charlie-83
Copy link
Contributor Author

This is a very simple PR. Hopefully should be quick to review

@dacap
Copy link
Member

dacap commented Jan 26, 2024

Hi @Charlie-83, I'll review this one today if I can.

@dacap dacap self-assigned this Jan 26, 2024
@dacap
Copy link
Member

dacap commented Jan 26, 2024

It works great @Charlie-83, I've just tested it, attaching an image here for future reference just in case:

diff

The first row is the gradient with the old version, and the second one with this patch 👍

dacap pushed a commit that referenced this pull request Jan 26, 2024
@dacap
Copy link
Member

dacap commented Jan 26, 2024

Merged in e47448c adding a reference to this PR in the commit message (I'll close the PR but actually it's merged).

@dacap dacap closed this Jan 26, 2024
@dacap
Copy link
Member

dacap commented Jan 26, 2024

Thanks for your contribution @Charlie-83!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants