Skip to content
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

Add compute_tile_highlight to foundation/ColorMap #2801

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add compute_tile_highlight to foundation/ColorMap #2801

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
18 changes: 18 additions & 0 deletions src/appleseed/foundation/image/colormap.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@

// Standard headers.
#include <algorithm>
#include <array>
#include <cassert>
#include <limits>

Expand Down Expand Up @@ -248,6 +249,23 @@ Color3f ColorMap::evaluate_palette(float x) const
return lerp(m_palette[ix], m_palette[ix + 1], w);
}

std::array<float,4> ColorMap::compute_tile_highlight_color(const size_t thread_index, const size_t thread_count)
{
// Choose one color per thread (see https://www.shadertoy.com/view/wlKXDm).
std::array<float,4> frame_color;

assert(thread_count != 0);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move first.


float t = 2 * 3.14159265359f * (thread_index / static_cast<float>(thread_count));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make all variables const.

float cos_t = std::cos(t);
float sin_t = std::sin(t);
frame_color[0] = 0.5f + (0.5f * cos_t);
frame_color[1] = 0.5f + (0.5f * (-0.5f * cos_t - 0.866f * sin_t));
frame_color[2] = 0.5f + (0.5f * (-0.5f * cos_t + 0.866f * sin_t));
frame_color[3] = 1.0f;
return frame_color;
}

AABB2u ColorMap::get_full_crop_window(const Image& image)
{
const CanvasProperties& props = image.properties();
Expand Down
7 changes: 7 additions & 0 deletions src/appleseed/foundation/image/colormap.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@
#include "foundation/image/color.h"
#include "foundation/math/aabb.h"

// appleseed.main headers.
#include "main/dllsymbol.h"

// Standard headers.
#include <cstddef>
#include <vector>
Expand Down Expand Up @@ -93,6 +96,10 @@ class ColorMap

Color3f evaluate_palette(float x) const;

APPLESEED_DLLSYMBOL std::array<float,4> compute_tile_highlight_color(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's not exactly how I saw it:

First, I would suggest to rename the function since it's not really specific to tile highlights. Maybe evaluate_cosine_palette()? There are many more of those by the way: https://www.shadertoy.com/view/ll2GD3, in the future we could consider adding more as needed.

Also, I think this should be a free function instead of static method as it's not related to the ColorMap class.

Finally, it would be more logical to return a Color3f.

const size_t thread_index,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we rename the method, those should be renamed as well (index, count sound good enough).

const size_t thread_count);

private:
std::vector<Color3f> m_palette;

Expand Down