Menu

#2619 CanvasAPI: setColorFill does not work on groups

2020.3
New
nobody
Low
2021-07-27
2021-07-24
legoboyvdlp
No

setColor works as expected on grouped objects from a .svg. However, when I run the .setColorFill command on a grouped object, it does not change the color of the grouped objects. This results in, for instance, the following: notice that the stroke is amber while the fill is green. Both should be amber.

To be clear, this rectangle element is created out of four lines in a .svg document. They are grouped together in Inkscape.

Discussion

  • legoboyvdlp

    legoboyvdlp - 2021-07-24
    • Description has changed:

    Diff:

    --- old
    +++ new
    @@ -1,3 +1,5 @@
     setColor works as expected on grouped objects from a .svg. However, when I run the .setColorFill command on a grouped object, it does not change the color of the grouped objects. This results in, for instance, the following: notice that the stroke is amber while the fill is green. Both should be amber. 
    
    +To be clear, this rectangle element is created out of four lines in a .svg document. They are grouped together in Inkscape.
    +
     ![](https://i.imgur.com/QAcPuYe.png)
    
     
  • James Turner

    James Turner - 2021-07-26

    This isn't a regression, correct? It's just a missing unimplemented feature in the style propogation of groups? Which is, to be honest, very incomplete.

     
  • legoboyvdlp

    legoboyvdlp - 2021-07-26

    Yep, it's never worked as long as I've used that code. It doesn't sound too hard to do; the .setColor is propagated but the fill feature isn't, if you could point me to where that's done?

     
  • James Turner

    James Turner - 2021-07-26

    From looking at the C++, actually /all/ style attributes should be propogated by the group to its direct children. So, it's weird.

    Can you test different style attr, and see if it's only setColor which works, or if it's only setColorFill which fails?

     
  • James Turner

    James Turner - 2021-07-26

    Oh, figured it out. So the C++ logic propogates all style values to children automatically.
    But in Nasal/canvas/api/group.nas, there is a 'manual' setColor which directly loops overs it children and sets the value on each.

    The C++ mechanism feels correct to me (since SVG uses CSS styling which should cascade down) : so I don't know why the Nasal 'setColor' was implemented that way.

    Does this help at all, or just make things more confusing?

     
  • legoboyvdlp

    legoboyvdlp - 2021-07-26

    No; that makes sense, and its simple to copy + paste that to setColorFill. I couldn't answer the question as to why its done that way; but to make it work maybe just duplicate it for setColorFill?

    That's much better!

     
  • James Turner

    James Turner - 2021-07-26

    Can you try setting the style directly on the group, rather than using setColor[Fill] APIs?

    What I'm worried about is that there is a generic feature which should be working everywhere, which would mean things like setColor were unnecessary. We /can/ add setColorFill etc but we end up with an other function for each style property, when none of them should be needed.

     
  • legoboyvdlp

    legoboyvdlp - 2021-07-26

    Looks to me like the element.nas code has to implement setColor and setColorFill too, so I'd imagine the group.nas code has to as well, except on grouped elements?

    Regardless; when I do this: obj["Pump-Blue"].set("fill", [0.0509,0.7529,0.2941]); it just does the same as before.

     
  • James Turner

    James Turner - 2021-07-27

    "Looks to me like the element.nas code has to implement setColor and setColorFill too, so I'd imagine the group.nas code has to as well, except on grouped elements?"

    No, the C++ code intent is that no such setColor/Foo/Etc should be needed on group at all.

    Basically, all style props on a group should cascade, just like normal CSS cascading. However, I wonder if the issue is that this would only apply if the stlye is cleared on the child objects. Let's make some simple tests for this, in Nasal.

     

Log in to post a comment.

MongoDB Logo MongoDB