[Bf-committers] [Bf-blender-cvs] SVN commit: /data/svn/bf-blender [59848] trunk/blender/source/blender: Code cleanup: use boolean instead of int for colormanagement
Sergey Sharybin
sergey.vfx at gmail.com
Sat Sep 14 18:31:59 CEST 2013
That would be rather much more crappy.
Two things here:
- When you've been working on a CM-related feature half of API using bool
and half of i using int is really annoying and totally unclear.
- Color management module was already mixing int<->bool casts in a not
actually safe way.
This is a reason why that change was needed (same goes to the tracking
module, but that was also using some weirdo int/double/float casts).
Further, per-file cleanup is not gonna to help you. Doing per-file cleanup
would only mean you'll either have some unsafe casts still or you'll end up
is so much spaghetti changes that you'll just make it so everyone who'll
try to merge them will go gardening instead.
Also, generally i'm avoiding global-ish cleanups, but there're cases when
not making things consistent all over is burying a dead cat under the code.
And the last thing, you couldn't suspend development in the trunk while
you're working in the branch and you'll later or sooner end up with the
same piece of code was changed in trunk and you wouldn't be able to avoid
checking what and why changed.
On Sat, Sep 14, 2013 at 2:39 AM, Dalai Felinto <dfelinto at gmail.com> wrote:
> Hi Sergey,
>
> Could we stick to doing cleanups per files instead of per 'categories'?
>
> For example image_ops.c was affected by this commit, but still has plenty
> of TRUEs.
> That means at some point someone else may change the same file for cleanup
> reasons.
>
> I think cleanups are important and all, but they can get in the way of
> people maintaining branches - thus the hassle should be minimized.
>
> (speaking for myself since this commit raised the potential commit conflict
> alert here leading me to see what changed, why changed, ...). It will be a
> pity if the same file gives branchers more work for the same reason in the
> future.
>
> Thanks,
> Dalai
>
> --
> blendernetwork.org/dalai-felinto
> www.dalaifelinto.com
>
>
> 2013/9/5 Sergey Sharybin <sergey.vfx at gmail.com>
>
> > Revision: 59848
> >
> >
> http://projects.blender.org/scm/viewvc.php?view=rev&root=bf-blender&revision=59848
> > Author: nazgul
> > Date: 2013-09-05 17:13:43 +0000 (Thu, 05 Sep 2013)
> > Log Message:
> > -----------
> > Code cleanup: use boolean instead of int for colormanagement
> >
> > Modified Paths:
> > --------------
> > trunk/blender/source/blender/blenkernel/intern/sequencer.c
> >
> > trunk/blender/source/blender/compositor/operations/COM_ImageOperation.cpp
> >
> >
> trunk/blender/source/blender/compositor/operations/COM_OutputFileOperation.cpp
> >
> >
> trunk/blender/source/blender/compositor/operations/COM_ViewerOperation.cpp
> > trunk/blender/source/blender/editors/render/render_internal.c
> > trunk/blender/source/blender/editors/render/render_opengl.c
> > trunk/blender/source/blender/editors/screen/glutil.c
> > trunk/blender/source/blender/editors/sculpt_paint/paint_image_proj.c
> > trunk/blender/source/blender/editors/space_image/image_ops.c
> > trunk/blender/source/blender/editors/space_sequencer/sequencer_draw.c
> > trunk/blender/source/blender/imbuf/IMB_colormanagement.h
> >
> trunk/blender/source/blender/imbuf/intern/IMB_colormanagement_intern.h
> > trunk/blender/source/blender/imbuf/intern/colormanagement.c
> > trunk/blender/source/blender/imbuf/intern/divers.c
> > trunk/blender/source/blender/makesrna/intern/rna_image_api.c
> > trunk/blender/source/blender/render/intern/source/pipeline.c
> > trunk/blender/source/blender/render/intern/source/render_result.c
> >
> > Modified: trunk/blender/source/blender/blenkernel/intern/sequencer.c
> > ===================================================================
> > --- trunk/blender/source/blender/blenkernel/intern/sequencer.c
> 2013-09-05
> > 16:32:44 UTC (rev 59847)
> > +++ trunk/blender/source/blender/blenkernel/intern/sequencer.c
> 2013-09-05
> > 17:13:43 UTC (rev 59848)
> > @@ -462,7 +462,7 @@
> >
> > if (!STREQ(float_colorspace, to_colorspace)) {
> >
> > IMB_colormanagement_transform_threaded(ibuf->rect_float, ibuf->x,
> ibuf->y,
> > ibuf->channels,
> > -
> > from_colorspace, to_colorspace, TRUE);
> > +
> > from_colorspace, to_colorspace, true);
> > }
> > }
> > }
> > @@ -477,7 +477,7 @@
> >
> > if (to_colorspace && to_colorspace[0] != '\0') {
> > IMB_colormanagement_transform_threaded(ibuf->rect_float,
> > ibuf->x, ibuf->y, ibuf->channels,
> > - from_colorspace,
> > to_colorspace, TRUE);
> > + from_colorspace,
> > to_colorspace, true);
> > }
> > }
> >
> >
> > Modified:
> > trunk/blender/source/blender/compositor/operations/COM_ImageOperation.cpp
> > ===================================================================
> > ---
> > trunk/blender/source/blender/compositor/operations/COM_ImageOperation.cpp
> > 2013-09-05 16:32:44 UTC (rev 59847)
> > +++
> > trunk/blender/source/blender/compositor/operations/COM_ImageOperation.cpp
> > 2013-09-05 17:13:43 UTC (rev 59848)
> > @@ -141,7 +141,7 @@
> > }
> > rgba_uchar_to_float(color, byte_color);
> > if (make_linear_rgb) {
> > -
> > IMB_colormanagement_colorspace_to_scene_linear_v4(color, FALSE,
> > ibuf->rect_colorspace);
> > +
> > IMB_colormanagement_colorspace_to_scene_linear_v4(color, false,
> > ibuf->rect_colorspace);
> > }
> > }
> > }
> >
> > Modified:
> >
> trunk/blender/source/blender/compositor/operations/COM_OutputFileOperation.cpp
> > ===================================================================
> > ---
> >
> trunk/blender/source/blender/compositor/operations/COM_OutputFileOperation.cpp
> > 2013-09-05 16:32:44 UTC (rev 59847)
> > +++
> >
> trunk/blender/source/blender/compositor/operations/COM_OutputFileOperation.cpp
> > 2013-09-05 17:13:43 UTC (rev 59848)
> > @@ -138,7 +138,7 @@
> > ibuf->mall |= IB_rectfloat;
> > ibuf->dither = this->m_rd->dither_intensity;
> >
> > - IMB_colormanagement_imbuf_for_write(ibuf, TRUE, FALSE,
> > m_viewSettings, m_displaySettings,
> > + IMB_colormanagement_imbuf_for_write(ibuf, true, false,
> > m_viewSettings, m_displaySettings,
> > this->m_format);
> >
> > BKE_makepicstring(filename, this->m_path, bmain->name,
> > this->m_rd->cfra, this->m_format,
> >
> > Modified:
> >
> trunk/blender/source/blender/compositor/operations/COM_ViewerOperation.cpp
> > ===================================================================
> > ---
> >
> trunk/blender/source/blender/compositor/operations/COM_ViewerOperation.cpp
> > 2013-09-05 16:32:44 UTC (rev 59847)
> > +++
> >
> trunk/blender/source/blender/compositor/operations/COM_ViewerOperation.cpp
> > 2013-09-05 17:13:43 UTC (rev 59848)
> > @@ -173,7 +173,7 @@
> > {
> > IMB_partial_display_buffer_update(this->m_ibuf,
> > this->m_outputBuffer, NULL, getWidth(), 0, 0,
> > this->m_viewSettings,
> > this->m_displaySettings,
> > - rect->xmin, rect->ymin,
> > rect->xmax, rect->ymax, FALSE);
> > + rect->xmin, rect->ymin,
> > rect->xmax, rect->ymax, false);
> >
> > this->updateDraw();
> > }
> >
> > Modified: trunk/blender/source/blender/editors/render/render_internal.c
> > ===================================================================
> > --- trunk/blender/source/blender/editors/render/render_internal.c
> > 2013-09-05 16:32:44 UTC (rev 59847)
> > +++ trunk/blender/source/blender/editors/render/render_internal.c
> > 2013-09-05 17:13:43 UTC (rev 59848)
> > @@ -164,7 +164,7 @@
> >
> > IMB_partial_display_buffer_update(ibuf, rectf, NULL, rr->rectx,
> > rxmin, rymin,
> > &scene->view_settings,
> > &scene->display_settings,
> > - rxmin, rymin, rxmin + xmax,
> > rymin + ymax, TRUE);
> > + rxmin, rymin, rxmin + xmax,
> > rymin + ymax, true);
> > }
> >
> > /* ****************************** render invoking ***************** */
> > @@ -1137,7 +1137,7 @@
> >
> > /* Try using GLSL display transform. */
> > if (force_fallback == false) {
> > - if (IMB_colormanagement_setup_glsl_draw(NULL,
> > &scene->display_settings, TRUE, FALSE)) {
> > + if (IMB_colormanagement_setup_glsl_draw(NULL,
> > &scene->display_settings, true, false)) {
> > glEnable(GL_BLEND);
> > glColor4f(1.0f, 1.0f, 1.0f, 1.0f);
> > glaDrawPixelsTex(rres.xof, rres.yof,
> > rres.rectx, rres.recty, GL_RGBA, GL_FLOAT,
> >
> > Modified: trunk/blender/source/blender/editors/render/render_opengl.c
> > ===================================================================
> > --- trunk/blender/source/blender/editors/render/render_opengl.c
> 2013-09-05
> > 16:32:44 UTC (rev 59847)
> > +++ trunk/blender/source/blender/editors/render/render_opengl.c
> 2013-09-05
> > 17:13:43 UTC (rev 59848)
> > @@ -591,7 +591,7 @@
> > ibuf_save = ibuf;
> >
> > if (is_movie ||
> > !BKE_imtype_requires_linear_float(scene->r.im_format.imtype)) {
> > - ibuf_save =
> > IMB_colormanagement_imbuf_for_write(ibuf, TRUE, TRUE,
> &scene->view_settings,
> > + ibuf_save =
> > IMB_colormanagement_imbuf_for_write(ibuf, true, true,
> &scene->view_settings,
> >
> > &scene->display_settings, &scene->r.im_format);
> >
> > needs_free = TRUE;
> >
> > Modified: trunk/blender/source/blender/editors/screen/glutil.c
> > ===================================================================
> > --- trunk/blender/source/blender/editors/screen/glutil.c
> 2013-09-05
> > 16:32:44 UTC (rev 59847)
> > +++ trunk/blender/source/blender/editors/screen/glutil.c
> 2013-09-05
> > 17:13:43 UTC (rev 59848)
> > @@ -1092,17 +1092,17 @@
> > if (ibuf->float_colorspace) {
> > ok =
> > IMB_colormanagement_setup_glsl_draw_from_space(view_settings,
> > display_settings,
> >
> > ibuf->float_colorspace,
> > -
> > TRUE, FALSE);
> > +
> > true, false);
> > }
> > else {
> > ok =
> > IMB_colormanagement_setup_glsl_draw(view_settings, display_settings,
> > -
> > TRUE, FALSE);
> > +
> > true, false);
> > }
> > }
> > else {
> > ok =
> > IMB_colormanagement_setup_glsl_draw_from_space(view_settings,
> > display_settings,
> >
> > ibuf->rect_colorspace,
> > -
> > FALSE, FALSE);
> > +
> > false, false);
> > }
> >
> > if (ok) {
> > @@ -1181,7 +1181,7 @@
> >
> > GPU_offscreen_bind(ofs);
> >
> > - if (!IMB_colormanagement_setup_transform_from_role_glsl(role,
> > TRUE)) {
> > + if (!IMB_colormanagement_setup_transform_from_role_glsl(role,
> > true)) {
> > GPU_offscreen_unbind(ofs);
> > GPU_offscreen_free(ofs);
> > return FALSE;
> >
> > Modified:
> > trunk/blender/source/blender/editors/sculpt_paint/paint_image_proj.c
> > ===================================================================
> > --- trunk/blender/source/blender/editors/sculpt_paint/paint_image_proj.c
> > 2013-09-05 16:32:44 UTC (rev 59847)
> > +++ trunk/blender/source/blender/editors/sculpt_paint/paint_image_proj.c
> > 2013-09-05 17:13:43 UTC (rev 59848)
> > @@ -3883,7 +3883,7 @@
> > float mask =
> > ((float)projPixel->mask) * (1.0f / 65535.0f);
> >
> >
> > straight_uchar_to_premul_float(newColor_f, projPixel->newColor.ch);
> > -
> > IMB_colormanagement_colorspace_to_scene_linear_v4(newColor_f, TRUE,
> > ps->reproject_ibuf->rect_colorspace);
> > +
> > IMB_colormanagement_colorspace_to_scene_linear_v4(newColor_f, true,
> > ps->reproject_ibuf->rect_colorspace);
> > mul_v4_v4fl(newColor_f,
> > newColor_f, mask);
> >
> >
> > blend_color_mix_float(projPixel->pixel.f_pt, projPixel->origColor.f,
> >
> > Modified: trunk/blender/source/blender/editors/space_image/image_ops.c
> > ===================================================================
> > --- trunk/blender/source/blender/editors/space_image/image_ops.c
> > 2013-09-05 16:32:44 UTC (rev 59847)
> > +++ trunk/blender/source/blender/editors/space_image/image_ops.c
> > 2013-09-05 17:13:43 UTC (rev 59848)
> > @@ -1336,7 +1336,7 @@
> > const char *relbase = ID_BLEND_PATH(CTX_data_main(C),
> > &ima->id);
> > const short relative = (RNA_struct_find_property(op->ptr,
> > "relative_path") && RNA_boolean_get(op->ptr, "relative_path"));
> > const short save_copy =
> (RNA_struct_find_property(op->ptr,
> > "copy") && RNA_boolean_get(op->ptr, "copy"));
> > - const short save_as_render =
> > (RNA_struct_find_property(op->ptr, "save_as_render") &&
> > RNA_boolean_get(op->ptr, "save_as_render"));
> > + const bool save_as_render =
> > (RNA_struct_find_property(op->ptr, "save_as_render") &&
> > RNA_boolean_get(op->ptr, "save_as_render"));
> > ImageFormatData *imf = &simopts->im_format;
> > short ok = FALSE;
> >
> > @@ -1362,7 +1362,7 @@
> > }
> > }
> >
> > - colormanaged_ibuf =
> > IMB_colormanagement_imbuf_for_write(ibuf, save_as_render, TRUE,
> > &imf->view_settings, &imf->display_settings, imf);
> > + colormanaged_ibuf =
> > IMB_colormanagement_imbuf_for_write(ibuf, save_as_render, true,
> > &imf->view_settings, &imf->display_settings, imf);
> >
> > if (simopts->im_format.imtype ==
> R_IMF_IMTYPE_MULTILAYER) {
> > Scene *scene = CTX_data_scene(C);
> >
> > Modified:
> > trunk/blender/source/blender/editors/space_sequencer/sequencer_draw.c
> > ===================================================================
> > --- trunk/blender/source/blender/editors/space_sequencer/sequencer_draw.c
> > 2013-09-05 16:32:44 UTC (rev 59847)
> > +++ trunk/blender/source/blender/editors/space_sequencer/sequencer_draw.c
> > 2013-09-05 17:13:43 UTC (rev 59848)
> > @@ -1086,10 +1086,10 @@
> > type = GL_FLOAT;
> >
> > if (ibuf->float_colorspace) {
> > - glsl_used =
> > IMB_colormanagement_setup_glsl_draw_from_space_ctx(C,
> > ibuf->float_colorspace, TRUE);
> > + glsl_used =
> > IMB_colormanagement_setup_glsl_draw_from_space_ctx(C,
> > ibuf->float_colorspace, true);
> > }
> > else {
> > - glsl_used =
> > IMB_colormanagement_setup_glsl_draw_ctx(C, TRUE);
> > + glsl_used =
> > IMB_colormanagement_setup_glsl_draw_ctx(C, true);
> > }
> > }
> > else if (ibuf->rect) {
> > @@ -1097,7 +1097,7 @@
> > format = GL_RGBA;
> > type = GL_UNSIGNED_BYTE;
> >
> > - glsl_used =
> > IMB_colormanagement_setup_glsl_draw_from_space_ctx(C,
> > ibuf->rect_colorspace, FALSE);
> > + glsl_used =
> > IMB_colormanagement_setup_glsl_draw_from_space_ctx(C,
> > ibuf->rect_colorspace, false);
> > }
> > else {
> > format = GL_RGBA;
> >
> > Modified: trunk/blender/source/blender/imbuf/IMB_colormanagement.h
> > ===================================================================
> >
> > @@ Diff output truncated at 10240 characters. @@
> > _______________________________________________
> > Bf-blender-cvs mailing list
> > Bf-blender-cvs at blender.org
> > http://lists.blender.org/mailman/listinfo/bf-blender-cvs
> >
> _______________________________________________
> Bf-committers mailing list
> Bf-committers at blender.org
> http://lists.blender.org/mailman/listinfo/bf-committers
>
--
With best regards, Sergey Sharybin
More information about the Bf-committers
mailing list