Skip to content

make sure notify(plot.color) changes the data #4676

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.

Sign up for GitHub

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

Merged
SimonDanisch merged 2 commits into from
Dec 17, 2024
Merged

make sure notify(plot.color) changes the data #4676

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fe4966f
make sure notify(plot.color) changes the data
SimonDanisch Dec 17, 2024
8374285
Update interfaces.jl
SimonDanisch Dec 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 10 additions & 5 deletions src/colorsampler.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,8 +300,15 @@ function _colormapping(
return Vec2f(apply_scale(scale, range))
end

color_scaled = lift(color_tight, colorscale; ignore_equal_values=true) do color, scale
return el32convert(apply_scale(scale, color))
color_scaled = Observable(el32convert(apply_scale(colorscale[], color_tight[])))
onany(color_tight, colorscale) do color, scale
scaled = el32convert(apply_scale(scale, color))
# If they're exactly the same we assume the user called notify
# So we trigger an update...
# If they're `== && !==`, we assume it's staying the same
if color_scaled[] === scaled || color_scaled[] != scaled
color_scaled[] = scaled
end
end
CT = ColorMapping{N,V,typeof(color_scaled[])}

Expand Down Expand Up @@ -335,11 +342,9 @@ function ColorMapping(
color_tight = Observable{T}(color)
# We need to copy, to check for changes
# Since users may re-use the array when pushing updates
last_colors = copy(color)
on(colors_obs) do new_colors
if new_colors !== last_colors
if color_tight[] === new_colors || color_tight[] != new_colors
color_tight[] = new_colors
last_colors = copy(new_colors)
end
end
# color_tight.ignore_equal_values = true
Expand Down
5 changes: 4 additions & 1 deletion src/interfaces.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,10 @@ function apply_expand_dimensions(trait, args, args_obs, deregister)
for (obs, arg) in zip(new_obs, expanded)
obs.val = arg
end
foreach(notify, new_obs)
# It should be enough to trigger the first observable since
# this will go into `map(convert_arguments, new_obs...)`
# Without this, we'll get 3 updates for `notify(data)` in `heatmap(data)`
notify(new_obs[1])
return
end
append!(deregister, fs)
Expand Down
68 changes: 68 additions & 0 deletions test/updating.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,3 +52,71 @@ end
@test glyph_collection_obs[][1].colors[1] == colors[1]
@test glyph_collection_obs[][end].colors[1] == colors[2]
end

@testset "updating heatmap with mutated array that is A === B" begin
n = 5
# Float32 is important, to not have a conversion inbetween, which circumvents A === B
data = Observable(fill(1f0, n, n))
s = Scene(; size=(200, 200))
# TODO heatmap!(s, data) triggers 3 times :(
hm = heatmap!(s, data)
color_triggered = Observable(0)
on(hm.calculated_colors[].color_scaled) do x
color_triggered[] += 1
end
colorrange_triggered = Observable(0)
on(hm.calculated_colors[].colorrange_scaled) do x
colorrange_triggered[] += 1
end
@test color_triggered[] == 0
notify(data)
@test color_triggered[] == 1
# If updating with a new array, that contains the same values, we don't want to trigger an updat
data[] = copy(data[])
@test color_triggered[] == 1
# Colorrange should never update if it stays the same
@test colorrange_triggered[] == 0
end

@testset "updating volume with mutated array that is A === B" begin
n = 5
# Float32 is important, to not have a conversion inbetween, which circumvents A === B
data = Observable(fill(1.0f0, n, n, n))
s = Scene(; size=(200, 200))
# TODO heatmap!(s, data) triggers 3 times :(
hm = volume!(s, data)
color_triggered = Observable(0)
on(hm.calculated_colors[].color_scaled) do x
color_triggered[] += 1
end
colorrange_triggered = Observable(0)
on(hm.calculated_colors[].colorrange_scaled) do x
colorrange_triggered[] += 1
end
@test color_triggered[] == 0
notify(data)
@test color_triggered[] == 1
# If updating with a new array, that contains the same values, we don't want to trigger an updat
data[] = copy(data[])
@test color_triggered[] == 1
# Colorrange should never update if it stays the same
@test colorrange_triggered[] == 0
end
#=
# Reference image test for the above, that I dont think is necessary
##
n = 5
# FLoat32 is important, so that it doesn't get converted
A = Observable(Float32.(Makie.peaks(n)));
s = Scene(size=(200, 200))
r = -0.75 .. 0.75
hm = heatmap!(s, r, r, A, colorrange=(-5, 8))
im1 = copy(colorbuffer(s))
A[] .= fill(0f0, n, n)
notify(A)
im2 = copy(colorbuffer(s))
large = vcat(im1, im2)
s = Scene(size=size(large));
image!(s, large; space=:pixel);
s
=#
Loading