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.

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 hint for unsample pixels #2069

Merged
merged 1 commit into from
May 31, 2018

Conversation

oktomus
Copy link
Member

@oktomus oktomus commented May 31, 2018

Highlight in red unsampled areas
2018-05-31-11 42 59
Pixels where on_pixel_end wasn't called. Usefull when implementing a new renderer

@@ -51,6 +51,7 @@ namespace renderer
// PixelRendererBase class implementation.
//

const uint8 NoSampleHint = 0;
const uint8 InvalidSampleHint = 1;
const uint8 CorrectSampleHint = 2;
Copy link
Member

Choose a reason for hiding this comment

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

Maybe it would be more logical that CorrectSampleHint is 0 and all problematic cases get non-zero values?

Also, can we find a better term than "hint"? These are not really hints, more like error codes...

Copy link
Member Author

Choose a reason for hiding this comment

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

0 is because the tile is filled with 0 by default. This avoid to do extra process.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, makes sense.

The comment about "hint" stands though.

@@ -116,6 +117,10 @@ void PixelRendererBase::on_tile_end(
tile.get_pixel(x, y, color);
color.rgb().set(0.2f * luminance(color.rgb())); // 20% of luminance
}
else if (sample_state[0] == NoSampleHint)
Copy link
Member

Choose a reason for hiding this comment

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

Simplify the logic, maybe with a switch where color gets assigned once in each case, instead of having a default value that later gets overwritten or altered.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes

@dictoon
Copy link
Member

dictoon commented May 31, 2018

Nice!

@oktomus oktomus force-pushed the minor-aov-samples-feature branch from bbe6b8b to 537cffd Compare May 31, 2018 11:45
@oktomus oktomus force-pushed the minor-aov-samples-feature branch from 537cffd to 702eea6 Compare May 31, 2018 12:16
@@ -51,8 +52,9 @@ namespace renderer
// PixelRendererBase class implementation.
//

const uint8 InvalidSampleHint = 1;
const uint8 CorrectSampleHint = 2;
const uint8 NoState = 0;
Copy link
Member

Choose a reason for hiding this comment

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

NoState? Did you mean NoSample?

Copy link
Member Author

Choose a reason for hiding this comment

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

The pixel can be both unsampled or unfinished. In my case, the pixel was sampled but pixel_end was never called.

@dictoon dictoon merged commit dbb18ee into appleseedhq:master May 31, 2018
@oktomus oktomus deleted the minor-aov-samples-feature branch July 10, 2018 06:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants