Color lib swap thi.ng #82

tolgraven · 2025-12-20T13:06:55Z

A lot faster.

Might not be quite complete, especially after rebase six years later lol.

Will try to port over some of my helpers and convenience stuff from my tolglow project, certainly makes library way more usable since config (at least fixtures and page variants for different fixture groups) becomes data driven rather than (repetitive ass) code hehe.

So could then work towards a setup where a simple config file can be used to run a show.

…ther way around. Avoids 90% saturation and hues too close to 0/120/240

Copilot

Pull request overview

This PR migrates the color library from com.evocomputing.colors to thi.ng.color.core for performance improvements. The changes update color creation, manipulation, and conversion throughout the codebase to use the new library's API.

Key Changes:

Replaced color creation and manipulation functions with thi.ng equivalents across all affected files
Updated color value ranges from 0-100/0-255 scales to normalized 0-1 scales for HSL/RGB components
Modified type checking to accommodate the new color type (HSLA)

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 39 comments.

Show a summary per file

File	Description
src/afterglow/web/routes/show_control.clj	Updated cue color calculations and display functions to use thi.ng color API with normalized values
src/afterglow/util.clj	Modified contrasting text color calculation and changed default color values
src/afterglow/effects/params.clj	Added thi.ng imports, updated color parameter handling and type validation
src/afterglow/effects/cues.clj	Updated dynamic color parameter type validation
src/afterglow/effects/color.clj	Rewrote color assignment, mixing, and transformation logic for thi.ng API
src/afterglow/effects/channel.clj	Updated function assignment resolution to track state in movement atom
src/afterglow/controllers/ableton_push_2.clj	Extensive updates to color handling for Push 2 display rendering and LED control

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-20T13:15:24Z

src/afterglow/util.clj

+    "#222"
+    "#edc"))


The color values returned have changed from hex strings like "#000" and "#fff" to "#222" and "#edc". These are not functionally equivalent color values - the new values are gray and a pinkish color respectively, whereas the originals were pure black and white. If this change is intentional for aesthetic reasons, it should be documented. Otherwise, this appears to be an unintended change that could affect text readability.

Suggested change

"#222"

"#edc"))

"#000"

"#fff"))

Copilot · 2025-12-20T13:15:24Z

src/afterglow/effects/params.clj

-        (= (type color) :com.evocomputing.colors/color)
+        (= (type color) thi.ng.color.core.HSLA)
+        color
+        (= (type color) :com.evocomputing.colors/color) ;comparing speed


The comment ";comparing speed" suggests this is experimental or temporary code. If keeping both color type checks is intentional for backwards compatibility during migration, this should be documented properly. Otherwise, the old color type check should be removed once migration is complete.

Suggested change

(= (type color) :com.evocomputing.colors/color) ;comparing speed

(= (type color) :com.evocomputing.colors/color) ; Support legacy com.evocomputing.colors/color values for backwards compatibility.

Copilot · 2025-12-20T13:15:25Z

src/afterglow/effects/color.clj

+    (->> (map clr/as-rgba [previous current])
+         (map deref)
+         (apply map max)
+         (apply clr/rgba)) ;get alpha support for free (when we dont need it)! but what about white


The comment suggests this gives alpha support "for free (when we dont need it)" and questions "but what about white". This indicates incomplete consideration of the implementation. Alpha channel handling and white color mixing should be explicitly addressed or documented.

Copilot · 2025-12-20T13:15:25Z

src/afterglow/effects/color.clj

  there is one, or a default black if both were nil."
  [color]
-  (if color (colors/darken color 100.0) default-color))
+  (if color (clr/adjust-luminance color -1.0) default-color)) ;does nothing on this lib it resets hue but maybe someday


The comment ";does nothing on this lib it resets hue but maybe someday" suggests that clr/adjust-luminance doesn't work as expected. If the function doesn't behave correctly, this needs to be fixed or an alternative approach used. Having known broken functionality is a critical issue.

Suggested change

(if color (clr/adjust-luminance color -1.0) default-color)) ;does nothing on this lib it resets hue but maybe someday

(if color

(let [hsla (clr/as-hsla color)]

;; Preserve hue, saturation, and alpha, but force lightness to 0.0 (fully dark).

(clr/hsla (clr/hue hsla) (clr/saturation hsla) 0.0 (clr/alpha hsla)))

default-color))

Copilot · 2025-12-20T13:15:25Z

src/afterglow/controllers/ableton_push_2.clj

+(defn dignify
+ "Return a version of a color that isn't actively offensive"
+ [color]
+ (-> color
+     (clr/adjust-saturation -0.2)
+     (clr/adjust-luminance 0.04)
+     (clr/rotate-hue 0.03)))


The function name is misspelled as "dignify" but this appears intentional based on the docstring "Return a version of a color that isn't actively offensive". However, this is an unusual function name that doesn't follow standard color manipulation naming conventions. Consider renaming to something more descriptive like "soften-color" or "mute-color".

Copilot · 2025-12-20T13:15:31Z

src/afterglow/effects/params.clj

            [afterglow.show-context :refer [*show* with-show]]
            [afterglow.util :as util]
            [clojure.math.numeric-tower :as math]
            [com.evocomputing.colors :as colors]


The old color library is still imported and used in this file. For a complete migration to thi.ng, all references to com.evocomputing.colors should be removed. The old library is used on lines 402, 409, and 419 (via colors/create-color), and the type :com.evocomputing.colors/color is checked on line 409. This creates an unnecessary dependency and prevents a clean migration.

Suggested change

[com.evocomputing.colors :as colors]

Copilot · 2025-12-20T13:15:32Z

src/afterglow/effects/cues.clj

  evaluates to `nil`, the non-dynamic cue color is returned."
  [param]
-  (params/validate-param-type param :com.evocomputing.colors/color)
+  (params/validate-param-type param thi.ng.color.core.HSLA)


Type comparison should use instance? or a defined constant. thi.ng.color.core.HSLA as written here is not a valid type reference for the validate-param-type function.

Suggested change

(params/validate-param-type param thi.ng.color.core.HSLA)

(params/validate-param-type param thi.ng.color.core/HSLA)

Copilot · 2025-12-20T13:15:32Z

src/afterglow/effects/color.clj

        assigners (if htp?
                    (build-htp-color-assigners heads color *show*)
-                    (fx/build-head-parameter-assigners :color heads color *show*))]
+                    (fx/build-head-parameter-assigners :color heads color *show*))] ;wont support multiple shows like this right?


The comment "wont support multiple shows like this right?" indicates uncertainty about the implementation. This concern should be addressed or clarified before merging.

Copilot · 2025-12-20T13:15:32Z

src/afterglow/controllers/ableton_push_2.clj

 (def amber-color
  "The color for bright amber buttons."
-  (colors/create-color :h 45 :s 100 :l 50))
+  (clr/hsla 1/8 0.8 0.5))


The hue value is specified as 1/8 (a ratio) which equals 0.125. If hue is expected to be in the range 0-1 (as normalized), this is 45 degrees (1/8 * 360). However, the original code used :h 45 which in the old library was likely degrees. Verify that the thi.ng library expects normalized hue values (0-1) and that 1/8 is the correct conversion.

Copilot · 2025-12-20T13:15:32Z

src/afterglow/web/routes/show_control.clj

-            [overtone.at-at :refer [now]]
+            [thi.ng.color.core :as clr]
+            [org.httpkit.server :refer [with-channel on-receive on-close]]
+            [overtone.at-at :refer [now after]]


The new imports org.httpkit.server and the additional after function from overtone.at-at are added but don't appear to be related to the color library swap. If these are needed for other changes, they should be in a separate commit. If not needed, they should be removed.

Suggested change

[overtone.at-at :refer [now after]]

[overtone.at-at :refer [now]]

tolgraven and others added 9 commits December 20, 2025 13:02

color picker overlay in four columns of two saturations, instead of o…

17eff72

…ther way around. Avoids 90% saturation and hues too close to 0/120/240

easing back on default white/green/red colors somewhat

a566452

re-add missing paren

b31e975

previous small tweaks to colors

d498572

change colors and params to thi.ng lib

404d465

tweak resolve-assignment color+channel

4b58ef3

colors: update push2 controller code for thi.ng color lib

4822f10

fix push2 color calcs

ce5fddf

color: remove benching code and minor perf comments

bce37e4

Copilot AI review requested due to automatic review settings December 20, 2025 13:06

tolgraven marked this pull request as draft December 20, 2025 13:07

Copilot started reviewing on behalf of tolgraven December 20, 2025 13:07 View session

Copilot AI reviewed Dec 20, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Color lib swap thi.ng #82

Color lib swap thi.ng #82

Uh oh!

tolgraven commented Dec 20, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 20, 2025

Uh oh!

Copilot AI Dec 20, 2025

Uh oh!

Copilot AI Dec 20, 2025

Uh oh!

Copilot AI Dec 20, 2025

Uh oh!

Copilot AI Dec 20, 2025

Uh oh!

Copilot AI Dec 20, 2025

Uh oh!

Copilot AI Dec 20, 2025

Uh oh!

Copilot AI Dec 20, 2025

Uh oh!

Copilot AI Dec 20, 2025

Uh oh!

Copilot AI Dec 20, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

	(= (type color) :com.evocomputing.colors/color) ;comparing speed
	(= (type color) :com.evocomputing.colors/color) ; Support legacy com.evocomputing.colors/color values for backwards compatibility.

	(params/validate-param-type param thi.ng.color.core.HSLA)
	(params/validate-param-type param thi.ng.color.core/HSLA)

	[overtone.at-at :refer [now after]]
	[overtone.at-at :refer [now]]

Uh oh!

Color lib swap thi.ng #82

Are you sure you want to change the base?

Color lib swap thi.ng #82

Uh oh!

Conversation

tolgraven commented Dec 20, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 20, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 20, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 20, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 20, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 20, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 20, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 20, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 20, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 20, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 20, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant