Skip to content

Commit

Permalink
simplifying histogram logic
Browse files Browse the repository at this point in the history
  • Loading branch information
@wkjarosz
wkjarosz committed Feb 26, 2024
1 parent 47f3fe3 commit f15a4ff
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 65 deletions.
16 changes: 1 addition & 15 deletions src/app.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1297,8 +1297,6 @@ void HDRViewApp::reset_tonemapping()
m_exposure_live = m_exposure = 0.0f;
m_gamma_live = m_gamma = 2.2f;
m_sRGB = true;
if (auto img = current_image())
img->update_selected_group_stats();
}

void HDRViewApp::normalize_exposure()
Expand All @@ -1311,7 +1309,6 @@ void HDRViewApp::normalize_exposure()
m = std::max(m, img->channels[group.channels[c]].get_stats()->maximum);

m_exposure_live = m_exposure = log2(1.f / m);
img->update_selected_group_stats();
}
}

Expand All @@ -1327,10 +1324,7 @@ void HDRViewApp::draw_top_toolbar()
ImGui::SetNextItemWidth(HelloImGui::EmSize(8));
ImGui::SliderFloat("##ExposureSlider", &m_exposure_live, -9.f, 9.f, "%5.2f");
if (ImGui::IsItemDeactivatedAfterEdit())
{
m_exposure = m_exposure_live;
img->update_selected_group_stats();
}

ImGui::SameLine();

Expand All @@ -1356,10 +1350,8 @@ void HDRViewApp::draw_top_toolbar()
ImGui::SetNextItemWidth(HelloImGui::EmSize(8));
ImGui::SliderFloat("##GammaSlider", &m_gamma_live, 0.02f, 9.f, "%5.3f");
if (ImGui::IsItemDeactivatedAfterEdit())
{
m_gamma = m_gamma_live;
img->update_selected_group_stats();
}

ImGui::EndDisabled();
ImGui::SameLine();

Expand Down Expand Up @@ -1516,15 +1508,9 @@ void HDRViewApp::process_hotkeys()
else if (ImGui::IsKeyPressed(ImGuiKey_Equal) || ImGui::IsKeyPressed(ImGuiKey_KeypadAdd))
zoom_in();
else if (ImGui::IsKeyPressed(ImGuiKey_E))
{
m_exposure_live = m_exposure += ImGui::IsKeyDown(ImGuiMod_Shift) ? 0.25f : -0.25f;
img->update_selected_group_stats();
}
else if (ImGui::IsKeyPressed(ImGuiKey_G))
{
m_gamma_live = m_gamma = std::max(0.02f, m_gamma + (ImGui::IsKeyDown(ImGuiMod_Shift) ? 0.02f : -0.02f));
img->update_selected_group_stats();
}
else if (ImGui::IsKeyPressed(ImGuiKey_F))
fit();
else if (ImGui::IsKeyPressed(ImGuiKey_C))
Expand Down
44 changes: 13 additions & 31 deletions src/image.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,20 +82,19 @@ void PixelStats::set_invalid()
average = 0.5f;
hist_x_scale = AxisScale_Linear;
hist_y_scale = AxisScale_Linear;
hist_x_limits = {0.f, 1.f};
hist_y_limits = {0.f, 1.f};
hist_normalization = {0.f, 1.f};
nan_pixels = 0;
inf_pixels = 0;
valid = false;
computed = false;
}

float2 PixelStats::x_limits(const Settings &settings) const
float2 PixelStats::x_limits(float e, AxisScale_ scale) const
{
bool LDR_scale = settings.x_scale == AxisScale_Linear || settings.x_scale == AxisScale_SRGB;
bool LDR_scale = scale == AxisScale_Linear || scale == AxisScale_SRGB;

float2 ret;
ret[1] = pow(2.f, -settings.exposure);
ret[1] = pow(2.f, -e);
if (minimum < 0.f)
ret[0] = -ret[1];
else
Expand Down Expand Up @@ -152,7 +151,7 @@ PixelStats::PixelStats(const Array2Df &img, float the_exposure, AxisScale_ x_sca

bool LDR_scale = x_scale == AxisScale_Linear || x_scale == AxisScale_SRGB;

hist_x_limits = x_limits(Settings{exposure, x_scale, y_scale});
auto hist_x_limits = x_limits(exposure, x_scale);

hist_normalization[0] = axis_scale_fwd_xform(LDR_scale ? hist_x_limits[0] : minimum, &x_scale);
hist_normalization[1] =
Expand Down Expand Up @@ -194,7 +193,7 @@ PixelStats::PixelStats(const Array2Df &img, float the_exposure, AxisScale_ x_sca

spdlog::trace("x_limits: {}; y_limits: {}", hist_x_limits, hist_y_limits);

valid = true;
computed = true;
}
catch (const std::exception &e)
{
Expand Down Expand Up @@ -254,14 +253,8 @@ Texture *Channel::get_texture()

bool PixelStats::Settings::match(const Settings &other) const
{
// spdlog::trace("checking match:\n\tother [{};{};{}];\n\tthis [{};{};{}]", other.exposure, other.x_scale,
// other.y_scale, exposure, x_scale, y_scale);
// fmt::print("checking needs_update {} {}\n", new_exposure, new_x_scale);
// when we use a logarithmic x-scale, we don't need to recompute if the exposure changes
// otherwise, we need to recompute if either the exposure changes or the x-scale changes.
return (other.x_scale == x_scale && other.exposure == exposure) ||
(other.x_scale == x_scale && (x_scale == AxisScale_SymLog || x_scale == AxisScale_Asinh));
// return other.x_scale == x_scale && other.y_scale == y_scale && other.exposure == exposure;
}

PixelStats *Channel::get_stats()
Expand All @@ -270,7 +263,7 @@ PixelStats *Channel::get_stats()

// We always return the cached stats, but before we do we might update the cache from the async stats

if (async_stats.ready() && async_stats.get()->valid)
if (async_stats.ready() && async_stats.get()->computed)
cached_stats = async_stats.get();

return cached_stats.get();
Expand Down Expand Up @@ -303,23 +296,22 @@ void Channel::update_stats()
};

// if the cached stats match and are valid, no need to recompute
if (cached_stats->settings().match(desired_settings) && cached_stats->valid)
if (cached_stats->settings().match(desired_settings) && cached_stats->computed)
return;

// cached stats are outdated, need to recompute

// if the async computation settings are outdated, or it was never computed -> recompute
if (!async_settings.match(desired_settings) || (async_stats.ready() && !async_stats.get()->valid))
if (!async_settings.match(desired_settings) || (async_stats.ready() && !async_stats.get()->computed))
{
recompute_async_stats();
return;
}

// check if we have
if (async_stats.ready() && async_stats.get()->valid)
// if the async computation is ready, grab it and possibly schedule again
if (async_stats.ready() && async_stats.get()->computed)
{
// replace cache with newer async stats
spdlog::trace("Transferring ownership of pixel stats");
cached_stats = async_stats.get();

// if these newer stats are still outdated, schedule a new async computation
Expand All @@ -328,17 +320,6 @@ void Channel::update_stats()
}
}

void Image::update_all_stats()
{
for (auto &c : channels) c.update_stats();
}

void Image::update_selected_group_stats()
{
for (int c = 0; c < groups[selected_group].num_channels; ++c)
channels[groups[selected_group].channels[c]].update_stats();
}

void Image::set_null_texture(Shader &shader, const string &target)
{
shader.set_uniform(fmt::format("{}_M_to_Rec709", target), float4x4{la::identity});
Expand Down Expand Up @@ -560,7 +541,8 @@ void Image::finalize()
channels.size(), num_channels)};
}

update_all_stats();
// update the stats/histograms for all channels
for (auto &c : channels) c.update_stats();
}

string Image::to_string() const
Expand Down
9 changes: 3 additions & 6 deletions src/image.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,15 +72,16 @@ struct PixelStats
float average = 0.0f;
int nan_pixels = 0;
int inf_pixels = 0;
bool valid = false; ///< Did we finish computing the stats?
bool computed = false; ///< Did we finish computing the stats?

// histogram
AxisScale_ hist_x_scale = AxisScale_Linear;
AxisScale_ hist_y_scale = AxisScale_Linear;
float2 hist_x_limits = {0.f, 1.f};
float2 hist_y_limits = {0.f, 1.f};
float2 hist_normalization = {0.f, 1.f};

float2 x_limits(float exposure, AxisScale_ x_scale) const;

std::array<float, NUM_BINS> hist_xs{};
std::array<float, NUM_BINS> hist_ys{};

Expand All @@ -106,8 +107,6 @@ struct PixelStats
return axis_scale_inv_xform(hist_normalization[1] * value * inv_bins + hist_normalization[0],
(void *)&hist_x_scale);
}

float2 x_limits(const Settings &settings) const;
};

struct Channel : public Array2Df
Expand Down Expand Up @@ -213,8 +212,6 @@ struct Image
void build_layers_and_groups();
void finalize();
std::string to_string() const;
void update_selected_group_stats();
void update_all_stats();

/**
Load the an image from the input stream.
Expand Down
20 changes: 8 additions & 12 deletions src/image_gui.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,8 @@ using namespace std;

void Image::draw_histogram()
{
static int bin_type = 1;
static ImPlotCond plot_cond = ImPlotCond_Always;
bool stats_need_update = false;
static int bin_type = 1;
static ImPlotCond plot_cond = ImPlotCond_Always;
{
const ImVec2 button_size = ImGui::IconButtonSize();
float combo_width =
Expand All @@ -44,8 +43,7 @@ void Image::draw_histogram()
ImGui::Text("X:");
ImGui::SameLine(0.f, ImGui::GetStyle().ItemInnerSpacing.x);
ImGui::SetNextItemWidth(combo_width);
if (ImGui::Combo("##X-axis type", &hdrview()->histogram_x_scale(), "Linear\0sRGB\0Asinh\0\0"))
stats_need_update = true;
ImGui::Combo("##X-axis type", &hdrview()->histogram_x_scale(), "Linear\0sRGB\0Asinh\0\0");

ImGui::SameLine();

Expand All @@ -64,19 +62,17 @@ void Image::draw_histogram()
string names[4];
auto colors = group.colors();

float2 x_limits = {std::numeric_limits<float>::infinity(), -std::numeric_limits<float>::infinity()};
float2 y_limits = x_limits;
PixelStats::Settings settings{hdrview()->exposure_live(), hdrview()->histogram_x_scale(),
hdrview()->histogram_y_scale()};
float2 x_limits = {std::numeric_limits<float>::infinity(), -std::numeric_limits<float>::infinity()};
float2 y_limits = x_limits;
for (int c = 0; c < std::min(3, group.num_channels); ++c)
{
auto &channel = channels[group.channels[c]];
if (stats_need_update)
channel.update_stats();
// if (stats_need_update)
channel.update_stats();
stats[c] = channel.get_stats();
y_limits[0] = std::min(y_limits[0], stats[c]->hist_y_limits[0]);
y_limits[1] = std::max(y_limits[1], stats[c]->hist_y_limits[1]);
auto xl = stats[c]->x_limits(settings);
auto xl = stats[c]->x_limits(hdrview()->exposure_live(), hdrview()->histogram_x_scale());
x_limits[0] = std::min(x_limits[0], xl[0]);
x_limits[1] = std::max(x_limits[1], xl[1]);
names[c] = Channel::tail(channel.name);
Expand Down
2 changes: 1 addition & 1 deletion src/imageio.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ static void copy_into_channel(Channel &channel, const float data[], int w, int h
{
constexpr bool dither = true;
parallel_for(0, h,
[&channel, n, c, w, &data, linearize, dither](int y)
[&channel, n, c, w, &data, linearize](int y)
{
for (int x = 0; x < w; ++x)
{
Expand Down

0 comments on commit f15a4ff

Please sign in to comment.