-
-
Notifications
You must be signed in to change notification settings - Fork 60
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
HCL bicone color distance function (without libfixmath) #104
Conversation
FYI, this currently comes in at just under +500 bytes on the move hub firmware. About 1/2 of the size is the new color constants and 1/2 is the new implementation. I don't seen any obvious ways to save a few more bytes (other than leaving out the |
I have confirmed the improved discrimination of black vs. dark gray. What other measurable benefits does the bicone method have over the existing code? |
This also helps with consistency in distinguishing bright colors from background colors. If we want to distinguish red from dark bluish gray, and add both colors to Since the weight of the hue-delta in the original cost function largely value- and saturation- independent (save for a cut-off), dark bluish gray would still sometimes be classified as red. We could change the cutoff, but any value will be a bit arbitrary and have some blind spots, so the most robust solution would be to weigh the hue linearly in chroma, which by itself is effectively the same as mapping the colors to a "prism" shape. If we then make the hue influence periodic, we get the HSV-cone. I have no intuition for the size cost on move hub, but I am guessing that 500 bytes is quite large. Do we have some tools to measure size cost or do I just have to look at the resulting binary? I can try to optimize it more. I could go back to a previous approach where I morphed the bicone to make it match the previous default colors. Will not be as robust with varying distance though and sacrifices some of the simplicity of the distance function. Would it be feasible to put this feature behind a |
Yes, it is printed when building the firmware:
(there is some variance in the size due to other factors, so consider this +/- 20 bytes of the "real" number)
We have recently freed up quite a bit of space on the move hub so 500 bytes isn't out of the question. If we leave out the |
c09c955
to
29698d3
Compare
I removed the This should be ready for review or merge if you guys are fine with it. |
29698d3
to
3b7e902
Compare
14c82b6
to
63f4839
Compare
Should be ready now. Also changed return 201 * x - x * x; to return (201 - x) * x; |
63f4839
to
b06bf29
Compare
Rebased for easier review on latest firmware. Review in progress :) |
This looks nice, thanks for submitting this! How do you feel about merging the new cost function, but leave it up to the user to add custom brick colors? Since the from pybricks.parameters import Color
Color.BRICK_YELLOW = Color(140, 89, 48) We can add an example in the docs with all available colors for users to pick from, including This saves space and we wouldn't be limited to only a small set of brick colors --- users would have to add color objects anyway. If that sounds OK to you I will go ahead and merge this. |
If you mean that the default colormap would be empty and the user would always provide his own detectable colors, that sounds good to me. However, if we went back to the default colormap with default Colors, as discussed in the previous PR (#93 (comment)), they are not reliably detected with realistic lego brick HSV values (Since e.g. realistic green is closer to |
A couple of other notes:
One issue seems to be that the euclidean distance hard-codes the relative weight of chroma and hue. |
Originally posted by @Novakasa in #93 (comment) Do you still have the formulas for this approach somewhere? Complexity in the cost function shouldn't be a major contributor to build size, so I'm curious about this one. |
For normalized S and V, the radial coordinate for each color would be
The Lightness (z-coordinate in the cone) can be calculated without division or additional multiplication only if it ranges from 0 to 10000. If we added an
Yes, though I have since moved on to processing the hsv values instead of using the sensor.color() method |
Thanks for the additional info. Don't worry about updating the PR right now though, I'm still reading through all the issues and trying to set up something to compare. Especially after realizing that
I don't think scaling the cone would make much of a difference. If I understand correctly, the original problem was mainly over-sensitivity to hue on low saturations/values. That's definitely something we can improve but based on some experiments I'm not sure that hard-coding the weight of hue relative to chroma is the best way to do it.
What does your new approach look like? Would it be better to just make the color map configurable? |
I'd also be curious to hear about specific problematic cases, so I can test them on my end too. E.g. I think you mentioned green bricks on a gray track with a black background. Is it also a problem stationary or just at speed? |
So my initial issue was like this: I had custom colors for the black background, the DBG tracks, and red and blue plates. I wanted to detect the red and blue plates and ignore the black and DBG colors. However, the color() method would sometimes return the red and blue color when it actually measured the DBG tracks. The reason for that is that the saturation fluctuates quite a bit for DBG, for the same reason that the saturation does not encode any real information for pure black. That's the reason that to me it would make the most sense to rather take into account the chroma value than the saturation. The chroma inherently encodes the fact that the darker a color is, the less perceptually important its saturation is. However, that way we can't control the saturation/value weight individually anymore, which is a problem when trying to associate colors with their S=V=100 counterpart, but for any cost function that actually wants to measure the perceptual distance of colors, this is a good approach. Once again, the core difficulty here is that we want a cost function that works well for custom calibrated colors as well as the default ones, which is why to me it seemed best to replace the default colors with realistic ones, but I am biased here since I only ever want to compare realistic colors.
My new approach is a quite simple filter based on the chroma value, and then checking the hue only for the filtered colors. It works very well so I doubt I would replace it with a colormap approach again, but if there would have been the possibility to provide custom color distance function I would have used that with success as well. |
b06bf29
to
e58b07f
Compare
Here is the extended PR: https://github.com/pybricks/pybricks-micropython/commits/hsv-bicone-int-2 Note that this keeps your main contribution unchanged, so this should remain a The only difference are a few cleanups in separate commits, as an alternate way to address the default colors. Curious to get your thoughts; happy to add my changes to your PR if you like. |
Thanks for giving this another chance! The timing is quite neat, as I have found some situations in my project where my previous approach for detecting markers needs some more sophistication. To have full control though, I might need access to the hsv cost function in a user program. Would this be too costly to implement? In either case, if this PR gets merged I can just do a draft PR to expose the cost function to see how expensive it is. Interesting idea to make None have negative value! If this makes the color detection robust with all default colors (Including green and white, which was problematic for me), this seems to be the best approach if we want to avoid adding calibrated colors. Unfortunately, I might have difficulty testing this currently, as I don't have all the colors available to me currently (my collection has been moved). Maybe I can find some LEGO around the house in a few hours. |
That could certainly be an interesting addition. I can have a look. Has your program ever needed any kind of hysteresis or other internal state or has instantaneous detection usually worked well enough? Also if you have found a better cost function since your original contribution, it would be easy enough to swap it in. |
My previous approach was to record all hues above a certain chroma threshold. Once chroma is below the threshold I would average over the hues to match against the "marker hues". This worked flawlessly in my experience. Recently, I had to relax this to match already the first hue after the chroma threshold is met (to reject certain colors), and now I seem to get color detection issues from time to time. I did not have time to investigate this in full detail yet, but it seems like the detection worked better when I averaged over multiple hue samples. The first sample might capture the "transition" between two colors, which is a possible reason for the degraded reliability in this approach. Writing this, I am honestly not sure why I now only take the first sample now, I might have had a good reason for this but maybe not, I need to check. (maybe writing this just solved my issues) |
I think the HCL bicone is the best function that is a good compromise between simplicity and perceptual consistency, the only alternative to me is the regular HSV cone, but I find it unintuitive that white is exactly between two colors with opposite hues and max chroma. I think that in the HSV cone, Green would also be closer to white than None, so then the |
Thanks for the input! Perhaps to achieve a similar effect, there could be some form of hysteresis. Here are two partially overlapping variants/ideas:
But since this PR changes (almost) nothing for the user, we could always start by getting this merged and building on the more advanced features later. Since you're positive about the suggestions, I'll update this PR with those changes so we can review and test it in more detail. |
ffb4e1b
to
50471d5
Compare
When the pull request finishes building in half an hour or so, you can find the ready-made firmware files here: #104 (comment) |
OK, I just did some testing using this build. I was using variations of this script: from pybricks.pupdevices import ColorDistanceSensor
from pybricks.parameters import Color, Port, Button
from pybricks.hubs import ThisHub
sensor = ColorDistanceSensor(Port.B)
last_color = None
counter = 0
hub = ThisHub()
LBG = Color(h=217, s=46, v=60)
DBG = Color(h=207, s=48, v=39)
Color.DBG = DBG
Color.LBG = LBG
sensor.detectable_colors([Color.RED, Color.GREEN, Color.BLACK, Color.BLUE, Color.YELLOW, Color.DBG, Color.WHITE, Color.LBG])
hub.system.set_stop_button(None)
while True:
color = sensor.color()
counter += 1
if color != last_color:
print(counter, color)
last_color = color
counter = 0
if hub.button.pressed():
print(sensor.hsv()) And everything seems to work as expected. I had to calibrate DBG and LBG carefully (measure at the same distance), because they mostly differ in the brightness, and the brightness is very sensitive to distance. (seems to be inverse square law, see also pybricks/support#116 (comment)) (uncalibrated) Green seems to be detected reliably, and None still works when pointing into the air. So, to me, this looks good! |
Thanks for the quick test! Glad to hear the basics are working for you. In the tutorials, when people change the detectable colors, I think we’ll want to recommend that they measure all their colors instead of combining a few measurements with some of the builtin colors, for optimal performance. With the updated map, it was able to distinguish between the dbg sleepers, my desk underneath, a red tile, and a blue tile, which is pretty neat. Thanks for this, and sorry it took so long! |
Btw, I was testing with city hub + ColorDistanceSensor |
Added a commit to move some of the math, and clean up some of the steps. The only algorithmic change is that I clamped the lower v bound at 0 for radius computations. It probably makes sense for the negative V to only affect the delta z. (Ran out of time for testing today.) |
It would be nice to also do a If we can get ballpark the same performance, that would be nice. We can adjust the |
For color detection applications, negative h can be used to increase the contrast between "no color" and any other color. For most other application, the value has a lower bound of 0, which is what this commit enforces.
This new distance function estimates the similarity of two hsv colors by calculating their euclidean distance when mapped into a Hue-Chroma-Lightness bicone. This is much more robust for realistic colors, especially when low saturation or low value is involved.
The newly introduced color cost function intrinsically deals with low s and v, so we don't need to artificially suppress them here.
5586d16
to
554a2dd
Compare
With the newly introduced color cost function, any measured value less than 50 results in Color.NONE, which makes it the predominant result. This reduces the distance range for the default colors. Instead of adjusting all colors (which are also used to emit colors), we can make the value of Color.NONE negative to achieve a similar result.
Also ensure we pick the proper sign for v in radial computations.
554a2dd
to
7d400e5
Compare
It's merged! 🎉 |
Thank you for seeing this through to the end! |
This replaces #93, opting not to use libfixmath.
Uses the euclidean distance between colors when mapped into this HCL-bicone:
I've tested this quite a bit and found it to be robust. It can distinguish between black and dark bluish gray and NONE very reliably.
I also added calibrated default colors with the
BRICK_
prefix for a few standard lego colors and put them as new defaults. For now, I've chosen to use colors measured at a distance of 2 LEGO plates (=6.4 mm), but I have collected a lot of data so we can use other distance as well. If we chose colors at a larger distance, only their value will be lower, which might make the distance window larger to detect these colors rather than a darker one.Thesensor.detectable_colors()
function now has a new kwargchroma_weight
which controls the relationship of relative height to width of the HCL-bicone used to estimate the color distance. Setting it to 0 would be equivalent seeing only black & white.I consider this ready to review, with the following notes:
chroma_weight
parameter so it might not be strictly necessary. Though to most users, it will have a useful default (50) and is entirely optional. EDIT: I removed it.BRICK_
colors need to be changed. As of now, if the hsv output of the other sensors is at least somewhat consistent with the ColorDistanceSensor, the results should be quite robust for the other sensors as well.