From e0c65512aad01e8da55b9b9c373ff1beb4c9d21e Mon Sep 17 00:00:00 2001 From: David Rose Date: Sat, 10 Jan 2009 01:19:16 +0000 Subject: [PATCH] saner treatment of ColorAttrib --- panda/src/pgraph/colorAttrib.I | 11 ++---- panda/src/pgraph/colorAttrib.cxx | 31 ++++++++++----- panda/src/pgraph/colorAttrib.h | 3 +- panda/src/pgraph/geomNode.cxx | 59 ++++++++++++---------------- panda/src/pgraph/geomTransformer.cxx | 49 ++++++----------------- 5 files changed, 65 insertions(+), 88 deletions(-) diff --git a/panda/src/pgraph/colorAttrib.I b/panda/src/pgraph/colorAttrib.I index 33137ed514..50656a169f 100644 --- a/panda/src/pgraph/colorAttrib.I +++ b/panda/src/pgraph/colorAttrib.I @@ -40,10 +40,7 @@ ColorAttrib(ColorAttrib::Type type, const Colorf &color) : // ColorAttrib for all geometry. You can get this // color via get_color(). // -// T_off - do not issue any color commands at all. -// This is generally used only in contexts where the -// color is meaningless, e.g. when drawing directly to -// the depth buffer. +// T_off - use the color white. //////////////////////////////////////////////////////////////////// INLINE ColorAttrib::Type ColorAttrib:: get_color_type() const { @@ -53,9 +50,9 @@ get_color_type() const { //////////////////////////////////////////////////////////////////// // Function: ColorAttrib::get_color // Access: Published -// Description: If the type is T_flat, this returns the color that -// will be applied to geometry. If the type is anything -// else, this is meaningless. +// Description: If the type is T_flat or T_off, this returns the +// color that will be applied to geometry. If the type +// is T_vertex, this is meaningless. //////////////////////////////////////////////////////////////////// INLINE const Colorf &ColorAttrib:: get_color() const { diff --git a/panda/src/pgraph/colorAttrib.cxx b/panda/src/pgraph/colorAttrib.cxx index 70600ba036..32961b5d0b 100644 --- a/panda/src/pgraph/colorAttrib.cxx +++ b/panda/src/pgraph/colorAttrib.cxx @@ -37,7 +37,7 @@ make_vertex() { if (_vertex != 0) { return _vertex; } - ColorAttrib *attrib = new ColorAttrib(T_vertex); + ColorAttrib *attrib = new ColorAttrib(T_vertex, Colorf::zero()); _vertex = return_new(attrib); return _vertex; } @@ -58,15 +58,14 @@ make_flat(const Colorf &color) { // Function: ColorAttrib::make_off // Access: Published, Static // Description: Constructs a new ColorAttrib object that indicates -// geometry should be rendered without any color -// commands at all. +// geometry should be rendered in white. //////////////////////////////////////////////////////////////////// CPT(RenderAttrib) ColorAttrib:: make_off() { if (_off != 0) { return _off; } - ColorAttrib *attrib = new ColorAttrib(T_off); + ColorAttrib *attrib = new ColorAttrib(T_off, Colorf(1.0f, 1.0f, 1.0f, 1.0f)); _off = return_new(attrib); return _off; } @@ -80,7 +79,7 @@ make_off() { //////////////////////////////////////////////////////////////////// CPT(RenderAttrib) ColorAttrib:: make_default() { - return make_flat(Colorf(1.0f, 1.0f, 1.0f, 1.0f)); + return make_off(); } //////////////////////////////////////////////////////////////////// @@ -143,10 +142,22 @@ compare_to_impl(const RenderAttrib *other) const { //////////////////////////////////////////////////////////////////// void ColorAttrib:: quantize_color() { - _color[0] = cfloor(_color[0] * 1000.0f + 0.5f) * 0.001f; - _color[1] = cfloor(_color[1] * 1000.0f + 0.5f) * 0.001f; - _color[2] = cfloor(_color[2] * 1000.0f + 0.5f) * 0.001f; - _color[3] = cfloor(_color[3] * 1000.0f + 0.5f) * 0.001f; + switch (_type) { + case T_flat: + _color[0] = cfloor(_color[0] * 1000.0f + 0.5f) * 0.001f; + _color[1] = cfloor(_color[1] * 1000.0f + 0.5f) * 0.001f; + _color[2] = cfloor(_color[2] * 1000.0f + 0.5f) * 0.001f; + _color[3] = cfloor(_color[3] * 1000.0f + 0.5f) * 0.001f; + break; + + case T_off: + _color.set(1.0f, 1.0f, 1.0f, 1.0f); + break; + + case T_vertex: + _color.set(0.0f, 0.0f, 0.0f, 0.0f); + break; + } } //////////////////////////////////////////////////////////////////// @@ -184,7 +195,7 @@ write_datagram(BamWriter *manager, Datagram &dg) { //////////////////////////////////////////////////////////////////// TypedWritable *ColorAttrib:: make_from_bam(const FactoryParams ¶ms) { - ColorAttrib *attrib = new ColorAttrib; + ColorAttrib *attrib = new ColorAttrib(T_off, Colorf::zero()); DatagramIterator scan; BamReader *manager; diff --git a/panda/src/pgraph/colorAttrib.h b/panda/src/pgraph/colorAttrib.h index df895f7985..6df7e0507b 100644 --- a/panda/src/pgraph/colorAttrib.h +++ b/panda/src/pgraph/colorAttrib.h @@ -34,8 +34,7 @@ PUBLISHED: }; private: - INLINE ColorAttrib(Type type = T_vertex, - const Colorf &color = Colorf(0.0f, 0.0f, 0.0f, 1.0f)); + INLINE ColorAttrib(Type type, const Colorf &color); PUBLISHED: static CPT(RenderAttrib) make_vertex(); diff --git a/panda/src/pgraph/geomNode.cxx b/panda/src/pgraph/geomNode.cxx index 9f0a8a4a20..e64111fa83 100644 --- a/panda/src/pgraph/geomNode.cxx +++ b/panda/src/pgraph/geomNode.cxx @@ -134,17 +134,12 @@ apply_attribs_to_vertices(const AccumulatedAttribs &attribs, int attrib_types, if ((attrib_types & SceneGraphReducer::TT_color) != 0) { CPT(RenderAttrib) ra = geom_attribs._color; - int override = geom_attribs._color_override; - if (ra == (const RenderAttrib *)NULL) { - // If we don't have a color attrib, implicitly apply the - // "off" attrib. But use an override of -1, so we don't - // replace a color attrib already on the Geom state. - ra = ColorAttrib::make_off(); - override = -1; + if (ra != (const RenderAttrib *)NULL) { + int override = geom_attribs._color_override; + entry->_state = entry->_state->add_attrib(ra, override); } - entry->_state = entry->_state->add_attrib(ra, override); - ra = entry->_state->get_attrib(ColorAttrib::get_class_slot()); + ra = entry->_state->get_attrib_def(ColorAttrib::get_class_slot()); const ColorAttrib *ca = DCAST(ColorAttrib, ra); if (ca->get_color_type() != ColorAttrib::T_vertex) { if (transformer.remove_column(new_geom, InternalName::get_color())) { @@ -160,30 +155,28 @@ apply_attribs_to_vertices(const AccumulatedAttribs &attribs, int attrib_types, // Now, if we have an "off" or "flat" color attribute, we // simply modify the color attribute, and leave the // vertices alone. - const RenderAttrib *ra = entry->_state->get_attrib(ColorAttrib::get_class_slot()); - if (ra != (const RenderAttrib *)NULL) { - const ColorAttrib *ca = DCAST(ColorAttrib, ra); - if (ca->get_color_type() == ColorAttrib::T_off) { - entry->_state = entry->_state->set_attrib(ColorAttrib::make_vertex()); - // ColorAttrib::T_off means the color scale becomes - // the new color. - entry->_state = entry->_state->set_attrib(ColorAttrib::make_flat(csa->get_scale())); - - } else if (ca->get_color_type() == ColorAttrib::T_flat) { - // ColorAttrib::T_flat means the color scale modulates - // the specified color to produce a new color. - const Colorf &c1 = ca->get_color(); - const LVecBase4f &c2 = csa->get_scale(); - Colorf color(c1[0] * c2[0], c1[1] * c2[1], - c1[2] * c2[2], c1[3] * c2[3]); - entry->_state = entry->_state->set_attrib(ColorAttrib::make_flat(color)); - - } else { - // Otherwise, we have vertex color, and we just scale - // it normally. - if (transformer.transform_colors(new_geom, csa->get_scale())) { - any_changed = true; - } + const RenderAttrib *ra = entry->_state->get_attrib_def(ColorAttrib::get_class_slot()); + const ColorAttrib *ca = DCAST(ColorAttrib, ra); + if (ca->get_color_type() == ColorAttrib::T_off) { + entry->_state = entry->_state->set_attrib(ColorAttrib::make_vertex()); + // ColorAttrib::T_off means the color scale becomes + // the new color. + entry->_state = entry->_state->set_attrib(ColorAttrib::make_flat(csa->get_scale())); + + } else if (ca->get_color_type() == ColorAttrib::T_flat) { + // ColorAttrib::T_flat means the color scale modulates + // the specified color to produce a new color. + const Colorf &c1 = ca->get_color(); + const LVecBase4f &c2 = csa->get_scale(); + Colorf color(c1[0] * c2[0], c1[1] * c2[1], + c1[2] * c2[2], c1[3] * c2[3]); + entry->_state = entry->_state->set_attrib(ColorAttrib::make_flat(color)); + + } else { + // Otherwise, we have vertex color, and we just scale + // it normally. + if (transformer.transform_colors(new_geom, csa->get_scale())) { + any_changed = true; } } } diff --git a/panda/src/pgraph/geomTransformer.cxx b/panda/src/pgraph/geomTransformer.cxx index a8b26210ba..5c71a86df7 100644 --- a/panda/src/pgraph/geomTransformer.cxx +++ b/panda/src/pgraph/geomTransformer.cxx @@ -819,26 +819,6 @@ remove_column(GeomNode *node, const InternalName *column) { // RenderStates the same. It does this by // canonicalizing the ColorAttribs, and in the future, // possibly other attribs. -// -// This implementation is not very smart yet. It -// unnecessarily canonicalizes ColorAttribs even if -// this will not yield compatible RenderStates. A -// better algorithm would: -// -// - each geom already starts with an original -// RenderState. In addition to this, calculate for -// each geom a canonical RenderState. -// -// - maintain a table mapping canonical RenderState -// to a list of geoms. -// -// - for each group of geoms with the same -// canonical renderstate, see if they already have -// matching RenderStates. -// -// - If they have differing RenderStates, then -// actually canonicalize the geoms. -// //////////////////////////////////////////////////////////////////// bool GeomTransformer:: make_compatible_state(GeomNode *node) { @@ -850,15 +830,16 @@ make_compatible_state(GeomNode *node) { GeomNode::GeomList::iterator gi; PT(GeomNode::GeomList) geoms = cdata->modify_geoms(); - // For each geom, calculate a canonicalized RenderState, and - // classify all the geoms according to that. + // For each geom, calculate a canonicalized RenderState, and + // classify all the geoms according to that. By "canonicalize" + // here, we simply mean removing the ColorAttrib. typedef pmap > StateTable; StateTable state_table; for (int i = 0; i < (int)geoms->size(); i++) { GeomNode::GeomEntry &entry = (*geoms)[i]; - CPT(RenderState) canon = entry._state->add_attrib(ColorAttrib::make_vertex(), -1); + CPT(RenderState) canon = entry._state->remove_attrib(ColorAttrib::get_class_slot()); state_table[canon].push_back(i); } @@ -871,7 +852,7 @@ make_compatible_state(GeomNode *node) { // If the geoms in the group already have the same RenderStates, // then nothing needs to be done to this group. - pvector &indices = (*si).second; + const pvector &indices = (*si).second; bool mismatch = false; for (int i = 1; i < (int)indices.size(); i++) { if ((*geoms)[indices[i]]._state != (*geoms)[indices[0]]._state) { @@ -884,15 +865,13 @@ make_compatible_state(GeomNode *node) { } // The geoms do not have the same RenderState, but they could, - // since their canonicalized states are the same. Canonicalize them. + // since their canonicalized states are the same. Canonicalize + // them, by applying the colors to the vertices. const RenderState *canon_state = (*si).first; for (int i = 0; i < (int)indices.size(); i++) { GeomNode::GeomEntry &entry = (*geoms)[indices[i]]; - const RenderAttrib *ra = entry._state->get_attrib(ColorAttrib::get_class_slot()); - if (ra == (RenderAttrib *)NULL) { - ra = ColorAttrib::make_off(); - } + const RenderAttrib *ra = entry._state->get_attrib_def(ColorAttrib::get_class_slot()); const ColorAttrib *ca = DCAST(ColorAttrib, ra); if (ca->get_color_type() == ColorAttrib::T_vertex) { // All we need to do is ensure that the geom has a color column. @@ -900,21 +879,19 @@ make_compatible_state(GeomNode *node) { PT(Geom) new_geom = entry._geom.get_read_pointer()->make_copy(); if (set_color(new_geom, Colorf(1,1,1,1))) { entry._geom = new_geom; - any_changed = true; } } } else { - Colorf c(1,1,1,1); - if (ca->get_color_type() == ColorAttrib::T_flat) { - c = ca->get_color(); - } + // A flat color (or "off", which is white). Set the vertices + // to the indicated flat color. + Colorf c = ca->get_color(); PT(Geom) new_geom = entry._geom.get_read_pointer()->make_copy(); if (set_color(new_geom, c)) { entry._geom = new_geom; - any_changed = true; } } - entry._state = canon_state; + entry._state = canon_state->add_attrib(ColorAttrib::make_vertex()); + any_changed = true; } }