Make the center control point of shapes visible on dark backgrounds

[Antonio Ospite]

Hi,

the newly added center control points of shapes currently have a fixed
stroke color of 0x010000000, the main problem with this is that the
control points become invisible on black backgrounds.

The fill and stroke colors for shape control points are hardcoded in
SPKnot::SPKnot(), and for the other control points the default values
work fine, for the NORMAL status they are:

  fill_color = 0xffffff00
  stroke_color = 0x01000000

That is white fill with a basically black stroke.

AFAICS when the alpha channel is 0 Inkscape does not apply the XOR
operator but uses the color as is (see
src/display/sodipodi-ctrl.cpp::sp_ctrl_render()) however the combination
from above still works fine on every background even without the XOR
compositing because of the heavy contrast between the stroke and the
fill color.

The problem with the center control points is that they are only drawn
with the stroke color, so XOR blending is actually needed for them.

An hackish solution would be to call SPKnot::setStroke() for any
entity_center->knot objects, like in the attached patch.

However this is not very elegant IMHO.

A better alternative would be to be able to pass a _set_ of colors for
the different states to KnotHolderEntity::create() which currently only
accepts a fill_color, but that would need some refactoring and I'd like
advice from developers more familiar with the Inkscape codebase than
I am.

Inkscape has different ways of specifying the control points color
depending on the state (normal, mouseover, dragging, selected).

guint32 arrays in src/seltrans-handles:

  guint32 const DEF_COLOR[] = { 0xff, 0xff6600, 0xff6600, 0xff, 0xff, 0xff };
  guint32 const CEN_COLOR[] = { 0x0, 0x0, 0x0, 0xff, 0xff0000b0, 0xff0000b0 };

Scalar values assigned individually like
SPKnot::setFill()/SPKnot::setStroke() in src/knot.cpp:

  void SPKnot::setFill(guint32 normal, guint32 mouseover, guint32 dragging, guint32 selected) {
      fill[SP_KNOT_STATE_NORMAL] = normal;
      fill[SP_KNOT_STATE_MOUSEOVER] = mouseover;
      fill[SP_KNOT_STATE_DRAGGING] = dragging;
      fill[SP_KNOT_STATE_SELECTED] = selected;
  }

And partial assignments, like in KnotHolderEntity::create() in
src/knot-holder-entity.cpp:

    knot->fill [SP_KNOT_STATE_NORMAL] = fill_color;
    g_object_set (G_OBJECT(knot->item), "fill_color", fill_color, NULL);

There is also a very handy structured data approach like
ControlPoint::ColorSet in src/ui/tool/node.cpp

  ControlPoint::ColorSet Node::node_colors = {
      {0xbfbfbf00, 0x000000ff}, // normal fill, stroke
      {0xff000000, 0x000000ff}, // mouseover fill, stroke
      {0xff000000, 0x000000ff}, // clicked fill, stroke
      //
      {0x0000ffff, 0x000000ff}, // normal fill, stroke when selected
      {0xff000000, 0x000000ff}, // mouseover fill, stroke when selected
      {0xff000000, 0x000000ff} // clicked fill, stroke when selected
  };

Ideally I would like to be able to pass a ColorSet to
KnotHolderEntity::create(), and possibly even to SPKnot::setFill()
SPKnot::setStoke().

Any advice about what the best place to move the ColorSet definition
would be?

BTW what is the relationship between ControlPoint and SPKnot?

Thanks,
   Antonio

[Antonio Ospite (ospite)]

Updated the attachment, the old one contained an unrelated change.

[Jabiertxof (jabiertxof)]

Hi Antonio
On Tue, 2017-08-01 at 14:07 +0000, Antonio Ospite wrote:
> Public bug reported:
>
> Hi,
>
> the newly added center control points of shapes currently have a
> fixed
> stroke color of 0x010000000, the main problem with this is that the
> control points become invisible on black backgrounds.
>
> The fill and stroke colors for shape control points are hardcoded in
> SPKnot::SPKnot(), and for the other control points the default values
> work fine, for the NORMAL status they are:
>
>   fill_color   = 0xffffff00
>   stroke_color = 0x01000000
>
> That is white fill with a basically black stroke.
>
> AFAICS when the alpha channel is 0 Inkscape does not apply the XOR
> operator but uses the color as is  (see
> src/display/sodipodi-ctrl.cpp::sp_ctrl_render()) however the
> combination
> from above still works fine on every background even without the XOR
> compositing because of the heavy contrast between the stroke and the
> fill color.
>
> The problem with the center control points is that they are only
> drawn
> with the stroke color, so XOR blending is actually needed for them.
>
> An hackish solution would be to call SPKnot::setStroke() for any
> entity_center->knot objects, like in the attached patch.
>
> However this is not very elegant IMHO.
>
> A better alternative would be to be able to pass a _set_ of colors
> for
> the different states to KnotHolderEntity::create() which currently
> only
> accepts a fill_color, but that would need some refactoring and I'd
> like
> advice from developers more familiar with the Inkscape codebase than
> I am.
>
> Inkscape has different ways of specifying the control points color
> depending on the state (normal, mouseover, dragging, selected).
>
> guint32 arrays in src/seltrans-handles:
>
>   guint32 const DEF_COLOR[] = { 0xff, 0xff6600, 0xff6600, 0xff, 0xff,
> 0xff };
>   guint32 const CEN_COLOR[] = { 0x0, 0x0, 0x0, 0xff, 0xff0000b0,
> 0xff0000b0 };
>

I think seltrans is not into the game, fixed colors could be ok

> Scalar values assigned individually like
> SPKnot::setFill()/SPKnot::setStroke() in src/knot.cpp:
>
>   void SPKnot::setFill(guint32 normal, guint32 mouseover, guint32
> dragging, guint32 selected) {
>       fill[SP_KNOT_STATE_NORMAL] = normal;
>       fill[SP_KNOT_STATE_MOUSEOVER] = mouseover;
>       fill[SP_KNOT_STATE_DRAGGING] = dragging;
>       fill[SP_KNOT_STATE_SELECTED] = selected;
>   }
>

this is what i need to call from create KnotHolderEntity::create()
instead next and current fill one

> And partial assignments, like in KnotHolderEntity::create() in
> src/knot-holder-entity.cpp:
>
>     knot->fill [SP_KNOT_STATE_NORMAL] = fill_color;
>     g_object_set (G_OBJECT(knot->item), "fill_color", fill_color,
> NULL);
>
> There is also a very handy structured data approach like
> ControlPoint::ColorSet in src/ui/tool/node.cpp
>
>   ControlPoint::ColorSet Node::node_colors = {
>       {0xbfbfbf00, 0x000000ff}, // normal fill, stroke
>       {0xff000000, 0x000000ff}, // mouseover fill, stroke
>       {0xff000000, 0x000000ff}, // clicked fill, stroke
>       //
>       {0x0000ffff, 0x000000ff}, // normal fill, stroke when selected
>       {0xff000000, 0x000000ff...

[Antonio Ospite (ospite)]

>> Ideally I would like to be able to pass a ColorSet to
>> KnotHolderEntity::create(), and possibly even to SPKnot::setFill()
>> SPKnot::setStoke().
>>
>> Any advice about what the best place to move the ColorSet definition
>> would be?
>>
> to me is better create a similar structure than color set in
> knot.cpp/h because ControlPoint is a particular type of knot focus on
> node editing. As far I undertand dev on control/selected points need
> different aproach than other knot staff, so feel free to create
> a structure on knot.cpp/h to pass to create and change the create
> fill by a setFill, setStroke from the new create argument/s

I'd like to factor out and reuse stuff if possible, but if nobody has
a better advice I will follow yours and create a _new_ structure.

Thanks,
   Antonio

P.S. it looks like that the launchpad web UI does not show full
messages when they are too long.

[Jonathan Hofinger (jhofinger)]

Hi - thanks for reporting this bug, I've manually migrated it (including your patch) to Inkscape's new bug tracker on GitLab, and closed it here.

Please feel free to file new bugs about the issues you're seeing at http://inkscape.org/report.

Moved to: https://gitlab.com/inkscape/inbox/-/issues/2289
Closed by: https://gitlab.com/jhofinger