NetLogo / Tortoise

Compiler and runtime engine for NetLogo models that runs in JavaScript 🐢
https://netlogoweb.org
Other
56 stars 27 forks source link

ColorModel.colorToRGB incorrectly removes alpha channel #240

Closed CIVITAS-John closed 2 years ago

CIVITAS-John commented 2 years ago

This issue is related to an issue in Turtle Universe. (In Chinese; please use your translator).

In https://github.com/NetLogo/Tortoise/master/engine/src/main/coffee/engine/core/colormodel.coffee, line 120:

color.slice(0, 3).map( (c) -> StrictMath.round(c) )

It is understandable as the colorToRGB function might assume the RGB format to be the correct output.

However, since it is used in https://github.com/NetLogo/Tortoise/master/engine/src/main/coffee/engine/core/turtle.coffee, line 415:

@_registerTurtleStamp(@xcor, @ycor, @_size, @_heading, ColorModel.colorToRGB(@_color), @_getShape(), mode.name)

When the turtle has an RGBA color and tries to stamp, the function will remove the alpha channel which leads to a bug in Turtle Universe. I am surprised, though, that NetLogo Web does not manifest this bug. How did you do it?

TheBizzle commented 2 years ago

Galapagos provides its own code for interfacing between the NetLogo color model as the CSS/RGBA color model. See here.

(Note that the code still ultimately relies on the Tortoise ColorModel, but just adds some convenience wrappers around it.)

CIVITAS-John commented 2 years ago

Get it. Hence, we need to change the colormodel.coffee, because it would strip me of the alpha channel which I need to use.

TheBizzle commented 2 years ago

No, I don't think so. You should simply make your own wrapper over colorToRGB (like what I linked), and have that wrapper preserve the alpha channel. There's no need for a change to Tortoise code. A color's alpha channel is not affected by any calculations that the NetLogo engine does.

CIVITAS-John commented 2 years ago

Speaking from the engine perspective, the current behavior of "trimming RGBA to RGB" in places where RGBA colors should be honored may not be correct. So we either want to make a new procedure called colorToRGBA and use it in turtle.coffee or we should allow colorToRGBA to keep the 4th channel.

See the original code I posted: @_registerTurtleStamp(@xcor, @ycor, @_size, @_heading, ColorModel.colorToRGB(@_color), @_getShape(), mode.name)

The fact is, it will correctly convert NL color to RGB; it will correctly carry over RGB; it will mistakenly strip RGBA of its A channel.

I will do a quick hack in TU, but I don't think this should be the expected behavior. And the fix is going to be very quick.

TheBizzle commented 2 years ago

All cases that I've tested have shown correct behavior on netlogoweb.org. However, it is true that stamps and pen lines were dropping transparency information. I was able to create a test that showed model diff mismatch between desktop and Tortoise in 76005970830fa180bd69bd10f550d3951706e9e2, which I fixed in e8ba529db68d2825b54876182b7b24dc259aa158.

I'm not aware of any outstanding issue here, so I'm closing this ticket.