Ticket #1018 (closed defect: fixed)

Opened 2 years ago

Last modified 12 months ago

HSL_to_RGB() in auto4 utils rounds incorrectly

Reported by: TheFluff Owned by: nielsm
Priority: low Milestone: 3.0.0
Component: Scripting Version: 2.1.7
Severity: minor Keywords:
Cc: mermaid.sea@… Platform: All
Sub Component:

Description (last modified by nielsm) (diff) 

12:29:36 < Lexica-chan> the RGB color (228,252,205) maps to the triple (90.63829787234043, 0.8867924528301887, 0.8960784313725491) in HSL.
12:30:24 < Lexica-chan> however, HSL_to_RGB(90.63829787234043, 0.8867924528301887, 0.8960784313725491) returns the values 227, 251, 205
12:32:03 < Lexica-chan> I've already looked at the HSL_to_RGB() function... the function itself is right, but its final step (flooring) is bad. rounding would be better. (returning the values without any modification might also be an option, leave it to the end-user to decide what to do)

Attachments

bug1018.patch Download (478 bytes) - added by nielsm 2 years ago.

Change History

comment:1 Changed 2 years ago by TheFluff

  • Cc mermaid.sea@… added

Changed 2 years ago by nielsm

comment:2 Changed 2 years ago by nielsm

Attached patch that should fix this. It should only be applied to the trunk to keep backwards-compatible behaviour in the 2.1.x line.
(I'm working on a branch myself atm so can't apply patch.)

comment:3 Changed 2 years ago by nielsm

  • Milestone changed from 2.1.8 to 2.2.0

Forgot to change the target milestone after the comment above.

comment:4 Changed 2 years ago by verm

  • Status changed from new to closed
  • Resolution set to fixed

(In [3622]) Commit jfs' patch to fix rounding errors as he doesn't have trunk going at the moment. Closes #1018.

comment:5 Changed 2 years ago by nielsm

  • Status changed from closed to reopened
  • Resolution fixed deleted
  • Description modified (diff)

Reopening this, I just noticed that the change actually breaks HSL_to_RGB by no longer multiplying the results by 255, so it suddenly returns RGB in range 0..1 instead of 0..255.

comment:6 Changed 2 years ago by gpower

Replace lines:

242	        r = math.floor(r * 255)
243	        g = math.floor(g * 255)
244	        b = math.floor(b * 255)

with

R=math.floor(R*255 + 0.5)
G=math.floor(G*255 + 0.5)
B=math.floor(B*255 + 0.5)

It should fix the rounding error

comment:7 Changed 18 months ago by verm

  • Milestone changed from 2.2.0 to 3.0.0

Bump 2.2.0 tickets to milestone:3.0.0 (2.2.0 is becoming 3.0.0)

comment:8 Changed 12 months ago by nielsm

  • Status changed from accepted to closed
  • Resolution set to fixed

(In [5354]) Fix #1018 properly, make sure the colour values are brought into 0-255 range and rounded.

Note: See TracTickets for help on using tickets.