From 9447b2d234394c1d966f77ed87271a3625a81cdd Mon Sep 17 00:00:00 2001 From: "Steinar H. Gunderson" Date: Sun, 28 Oct 2012 19:58:23 +0100 Subject: [PATCH] Add (safe) asserts around all Effect::set_* return values, and add warnings for not checking them. --- blur_effect.cpp | 4 ++-- demo.cpp | 4 ++-- effect.h | 8 ++++---- effect_chain.cpp | 18 +++++++++--------- glow_effect.cpp | 8 ++++---- resample_effect.cpp | 4 ++-- unsharp_mask_effect.cpp | 4 ++-- util.h | 13 +++++++++++++ 8 files changed, 38 insertions(+), 25 deletions(-) diff --git a/blur_effect.cpp b/blur_effect.cpp index ec61aea..80ccb3f 100644 --- a/blur_effect.cpp +++ b/blur_effect.cpp @@ -16,9 +16,9 @@ BlurEffect::BlurEffect() { // The first blur pass will forward resolution information to us. hpass = new SingleBlurPassEffect(this); - hpass->set_int("direction", SingleBlurPassEffect::HORIZONTAL); + CHECK(hpass->set_int("direction", SingleBlurPassEffect::HORIZONTAL)); vpass = new SingleBlurPassEffect(NULL); - vpass->set_int("direction", SingleBlurPassEffect::VERTICAL); + CHECK(vpass->set_int("direction", SingleBlurPassEffect::VERTICAL)); update_radius(); } diff --git a/demo.cpp b/demo.cpp index 2243f2d..5bcfa43 100644 --- a/demo.cpp +++ b/demo.cpp @@ -222,8 +222,8 @@ int main(int argc, char **argv) //vignette_effect->set_float("inner_radius", inner_radius); //vignette_effect->set_vec2("center", (float[]){ 0.7f, 0.5f }); - diffusion_effect->set_float("radius", blur_radius); - diffusion_effect->set_float("blurred_mix_amount", blurred_mix_amount); + CHECK(diffusion_effect->set_float("radius", blur_radius)); + CHECK(diffusion_effect->set_float("blurred_mix_amount", blurred_mix_amount)); input->set_pixel_data(src_img); chain.render_to_screen(); diff --git a/effect.h b/effect.h index db0fa94..21068ef 100644 --- a/effect.h +++ b/effect.h @@ -179,10 +179,10 @@ public: // Set a parameter; intended to be called from user code. // Neither of these take ownership of the pointer. - virtual bool set_int(const std::string&, int value); - virtual bool set_float(const std::string &key, float value); - virtual bool set_vec2(const std::string &key, const float *values); - virtual bool set_vec3(const std::string &key, const float *values); + virtual bool set_int(const std::string&, int value) MUST_CHECK_RESULT; + virtual bool set_float(const std::string &key, float value) MUST_CHECK_RESULT; + virtual bool set_vec2(const std::string &key, const float *values) MUST_CHECK_RESULT; + virtual bool set_vec3(const std::string &key, const float *values) MUST_CHECK_RESULT; protected: // Register a parameter. Whenever set_*() is called with the same key, diff --git a/effect_chain.cpp b/effect_chain.cpp index d1654e5..5b5e367 100644 --- a/effect_chain.cpp +++ b/effect_chain.cpp @@ -259,7 +259,7 @@ Phase *EffectChain::compile_glsl_program( for (unsigned i = 0; i < effects.size(); ++i) { Node *node = effects[i]; if (node->effect->num_inputs() == 0) { - node->effect->set_int("needs_mipmaps", input_needs_mipmaps); + CHECK(node->effect->set_int("needs_mipmaps", input_needs_mipmaps)); } } frag_shader += std::string("#define INPUT ") + effects.back()->effect_id + "\n"; @@ -726,8 +726,8 @@ void EffectChain::fix_internal_color_spaces() continue; } Node *conversion = add_node(new ColorspaceConversionEffect()); - conversion->effect->set_int("source_space", input->output_color_space); - conversion->effect->set_int("destination_space", COLORSPACE_sRGB); + CHECK(conversion->effect->set_int("source_space", input->output_color_space)); + CHECK(conversion->effect->set_int("destination_space", COLORSPACE_sRGB)); conversion->output_color_space = COLORSPACE_sRGB; insert_node_between(input, conversion, node); } @@ -760,8 +760,8 @@ void EffectChain::fix_output_color_space() Node *output = find_output_node(); if (output->output_color_space != output_format.color_space) { Node *conversion = add_node(new ColorspaceConversionEffect()); - conversion->effect->set_int("source_space", output->output_color_space); - conversion->effect->set_int("destination_space", output_format.color_space); + CHECK(conversion->effect->set_int("source_space", output->output_color_space)); + CHECK(conversion->effect->set_int("destination_space", output_format.color_space)); conversion->output_color_space = output_format.color_space; connect_nodes(output, conversion); propagate_gamma_and_color_space(); @@ -841,7 +841,7 @@ void EffectChain::fix_internal_gamma_by_asking_inputs(unsigned step) } for (unsigned i = 0; i < nonlinear_inputs.size(); ++i) { - nonlinear_inputs[i]->effect->set_int("output_linear_gamma", 1); + CHECK(nonlinear_inputs[i]->effect->set_int("output_linear_gamma", 1)); nonlinear_inputs[i]->output_gamma_curve = GAMMA_LINEAR; } @@ -879,7 +879,7 @@ void EffectChain::fix_internal_gamma_by_inserting_nodes(unsigned step) if (node->incoming_links.empty()) { assert(node->outgoing_links.empty()); Node *conversion = add_node(new GammaExpansionEffect()); - conversion->effect->set_int("source_curve", node->output_gamma_curve); + CHECK(conversion->effect->set_int("source_curve", node->output_gamma_curve)); conversion->output_gamma_curve = GAMMA_LINEAR; connect_nodes(node, conversion); } @@ -893,7 +893,7 @@ void EffectChain::fix_internal_gamma_by_inserting_nodes(unsigned step) continue; } Node *conversion = add_node(new GammaExpansionEffect()); - conversion->effect->set_int("source_curve", input->output_gamma_curve); + CHECK(conversion->effect->set_int("source_curve", input->output_gamma_curve)); conversion->output_gamma_curve = GAMMA_LINEAR; insert_node_between(input, conversion, node); } @@ -928,7 +928,7 @@ void EffectChain::fix_output_gamma() Node *output = find_output_node(); if (output->output_gamma_curve != output_format.gamma_curve) { Node *conversion = add_node(new GammaCompressionEffect()); - conversion->effect->set_int("destination_curve", output_format.gamma_curve); + CHECK(conversion->effect->set_int("destination_curve", output_format.gamma_curve)); conversion->output_gamma_curve = output_format.gamma_curve; connect_nodes(output, conversion); } diff --git a/glow_effect.cpp b/glow_effect.cpp index a477505..064cdac 100644 --- a/glow_effect.cpp +++ b/glow_effect.cpp @@ -12,10 +12,10 @@ GlowEffect::GlowEffect() cutoff(new HighlightCutoffEffect), mix(new MixEffect) { - blur->set_float("radius", 20.0f); - mix->set_float("strength_first", 1.0f); - mix->set_float("strength_second", 1.0f); - cutoff->set_float("cutoff", 0.2f); + CHECK(blur->set_float("radius", 20.0f)); + CHECK(mix->set_float("strength_first", 1.0f)); + CHECK(mix->set_float("strength_second", 1.0f)); + CHECK(cutoff->set_float("cutoff", 0.2f)); } void GlowEffect::rewrite_graph(EffectChain *graph, Node *self) diff --git a/resample_effect.cpp b/resample_effect.cpp index c0afe0b..e1e6199 100644 --- a/resample_effect.cpp +++ b/resample_effect.cpp @@ -103,9 +103,9 @@ ResampleEffect::ResampleEffect() // The first blur pass will forward resolution information to us. hpass = new SingleResamplePassEffect(this); - hpass->set_int("direction", SingleResamplePassEffect::HORIZONTAL); + CHECK(hpass->set_int("direction", SingleResamplePassEffect::HORIZONTAL)); vpass = new SingleResamplePassEffect(NULL); - vpass->set_int("direction", SingleResamplePassEffect::VERTICAL); + CHECK(vpass->set_int("direction", SingleResamplePassEffect::VERTICAL)); update_size(); } diff --git a/unsharp_mask_effect.cpp b/unsharp_mask_effect.cpp index 545910f..0ca22ee 100644 --- a/unsharp_mask_effect.cpp +++ b/unsharp_mask_effect.cpp @@ -11,8 +11,8 @@ UnsharpMaskEffect::UnsharpMaskEffect() : blur(new BlurEffect), mix(new MixEffect) { - mix->set_float("strength_first", 1.0f); - mix->set_float("strength_second", -0.3f); + CHECK(mix->set_float("strength_first", 1.0f)); + CHECK(mix->set_float("strength_second", -0.3f)); } void UnsharpMaskEffect::rewrite_graph(EffectChain *graph, Node *self) diff --git a/util.h b/util.h index 082d114..b01d62c 100644 --- a/util.h +++ b/util.h @@ -49,4 +49,17 @@ void combine_two_samples(float w1, float w2, float *offset, float *total_weight, #define check_error() { int err = glGetError(); if (err != GL_NO_ERROR) { printf("GL error 0x%x at %s:%d\n", err, __FILE__, __LINE__); exit(1); } } #endif +#ifdef __GNUC__ +#define MUST_CHECK_RESULT __attribute__((warn_unused_result)) +#else +#define MUST_CHECK_RESULT +#endif + +// CHECK() is like assert(), but retains any side effects no matter the compilation mode. +#ifdef NDEBUG +#define CHECK(x) (void)(x) +#else +#define CHECK(x) do { bool ok = x; if (!ok) { fprintf(stderr, "%s:%d: %s: Assertion `%s' failed.\n", __FILE__, __LINE__, __PRETTY_FUNCTION__, #x); abort(); } } while (false) +#endif + #endif // !defined(_UTIL_H) -- 2.39.5