trunk: updated randomize_color extension also changes color / hue / saturation if set to 0

Bug #1579939 reported by Hachmann
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Inkscape
Fix Released
Low
Hachmann

Bug Description

Due to a "1 + " (which seems to be used to avoid division by zero) in the .py file, the extension also changes colors when randomize is set to 0% for H/S/L and the option is checked (user error).

This change removes the necessity for additional checkboxes altogether and relies on the sliders only, thus simplifying the interface, and making the extension a little bit more foolproof (and in those cases where a user would previously have checked a box and set the value to zero, also a tiny bit faster).

(I don't have a trunk branch, so I'm attaching the files as a zip - hope that's okay in this case?)

Revision history for this message
Hachmann (marenhachmann) wrote :
jazzynico (jazzynico)
tags: added: color extensions-plugins
Revision history for this message
jazzynico (jazzynico) wrote :

Patch tested successfully on Windows 7, Inkscape trunk rev. 14880.
(I'll try to commit it later today on my main dev computer.)

Changed in inkscape:
assignee: nobody → Hachmann (marenhachmann)
importance: Undecided → Low
milestone: none → 0.92
status: New → In Progress
Revision history for this message
Jabiertxof (jabiertxof) wrote :

Great UI improvement!

Revision history for this message
jazzynico (jazzynico) wrote :

Note to self:
add "if __name__ == '__main__'" before "c = C()"

Revision history for this message
Hachmann (marenhachmann) wrote :

I thought about this a bit more - if possible, I'd like to also add a randomization for the alpha value (not sure if that would be possible without changing the imported ColorEffect, need to look), and simplify it a bit by using a for loop.

Please wait with committing, jazzynico.

Revision history for this message
Hachmann (marenhachmann) wrote :

So this is my try at adding opacity to the mix.

As this should work for clones (which are probably a main target of the extension), I couldn't modify the single attributes, like stroke-opacity - they don't make a difference for clones (didn't know before I tried...).

So I resorted to modifying the whole object's opacity (in the style attribute). This has one disadvantage: opacity effects add up for grouped objects, as they are applied to the group, *and* to the objects within.

It still appears to be useful to me, and other color extensions also still seem to work with the modifications.
Please consider including this additional change, if it makes sense to you.

(also added jazzynico's __main__ thing)

Revision history for this message
su_v (suv-lp) wrote :

Hachmann wrote:
> I couldn't modify the single attributes, like stroke-opacity - they
> don't make a difference for clones

They do make a difference for clones (including stroke-opacity). The specific style property however needs to be unset (i.e. not present in the style attribute) for the original object (in the SVG source).

--
Caveat: I neither tested one of the modified extensions, nor locally diffed it against trunk to see an overview of the proposed changes.

Revision history for this message
Hachmann (marenhachmann) wrote :

Ah, thanks, su_v :)

Then the reason seems to be that Inkscape seems to always set them, even if not needed..., right?

Revision history for this message
Hachmann (marenhachmann) wrote :

I overlooked the changed FSF address in the trunk version... :/ Update attached. Sorry for the noise.

Revision history for this message
su_v (suv-lp) wrote :

Just curious - what is the reason for this change?

-#!/usr/bin/env python
+#! /usr/bin/python

Attaching diff of latest 'color_randomize_w_opacity2.zip' against current trunk (r14882) for easier review of the proposed changes.

Revision history for this message
su_v (suv-lp) wrote :

Hachmann wrote:
> Then the reason seems to be that Inkscape seems to always set them,
> even if not needed..., right?

It probably depends (e.g. on the tool used, and the tool's prefs for style, and the 'last used' desktop style) ... If you unset e.g. a stroke via GUI (the '?' in 'Fill & Stroke > Stroke paint'), the default properties defining details of the stroke are removed i.e. 'Unset' (just tested with Inkscape 0.48.5, 0.91 and trunk). If on the other hand you remove a stroke with 'X', they are retained, and 'stroke' is set to 'none' (in this case no clone can add / render a visible stroke anyway, because the original already defines 'stroke' as 'none').

Revision history for this message
su_v (suv-lp) wrote :

... and there's one more detail: if you add a stroke via palette (Shift+click), it does neither add nor update the stroke-opacity. OTOH adding/editing a stroke paint via Fill & Stroke does always set RGB color as well the paint server's opacity (there are four sliders, you can't set them individually in the dialog).

Revision history for this message
Hachmann (marenhachmann) wrote :

Then maybe this could be the reason why I didn't see any difference on my clones:
fill-opacity is not removed if color is unset... I didn't expect there to be a difference, sorry.

At least I (think I) can't rely on them all not being there with the extension.

> Just curious - what is the reason for this change?

Seems my 'manual' diff from current stable to trunk wasn't perfect. (The change was from 2011, I didn't think that had to be included... now it is) Thank you for checking!

Revision history for this message
jazzynico (jazzynico) wrote :

The new version looks very good!

I'm going to try it on Xubuntu later and commit if it's ok.

For your information, a color_randomize.test.py file is in progress so that we can run unit tests on the colmod and opacmod functions.

Revision history for this message
jazzynico (jazzynico) wrote :

New version (and unit test file) added in the trunk rev. 14884).
Thanks Maren!

Changed in inkscape:
status: In Progress → Fix Committed
Revision history for this message
Hachmann (marenhachmann) wrote :

Thank you, jazzynico (esp. for writing the tests!)!

Bryce Harrington (bryce)
Changed in inkscape:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.