Hi Scott, good idea and nice work. I agree that this is a nice improvement to the way vanishing point assistants are visualized!

Having applied the patch I do have a few notes though:

1.) Right now changes in density don't show up until you mouse over the canvas, change the opacity, etc. Not a big deal, but it'd feel a bit nicer if the canvas refreshed immediately when the slider is changed (when the value is set, that is).

2.) Hiding/Deactivating the assistant (clicking on the little eye) doesn't seem to hide the reference lines for me.

3.) While small, in my opinion it'd be nice for user feedback if the currently selected assistant was visualized in some way while they're using the Assistants Tool. For example, maybe changing the color of the assistant itself, or maybe changing the color of the assistant's controller 'gizmo' - just some small way of showing the user "you've clicked on this thing and now it's selected". You know?

At any rate, it looks nice!

Working very nicely now! Density updates responsively, hiding works, and the selection highlighting is clear and looks great.

Good job again, Scott. =]

On my system I should apply this patch to make it build:

I have built it, but didn't test yet :)

Thanks for the review @dkazakov

I am hitting a snag trying to refactor it. All of the XML loading and saving for the assistant is done in the base class of the painting assistant, so I am not sure if I can use derived classes in a base class when saving a property that only exists in one derived class. See this situation in the code when saving the XML...

QByteArray KisPaintingAssistant::saveXml(QMap<KisPaintingAssistantHandleSP, int> &handleMap)
{

QByteArray data;
QXmlStreamWriter xml(&data);
xml.writeStartDocument();
xml.writeStartElement("assistant");
xml.writeAttribute("type",d->id);
xml.writeAttribute("active", QString::number(d->isSnappingActive));


if (d->id == "vanishingpoint") {
    VanishingPoingAssistant vpAssistant;  // having a hard time doing this since Vanishing point derives from this class. I would have to re-implemet the saving and loading XML stuff elsewhere
}

I am having a hard time typecasting or referencing the vanishing point assistant type to get its special property.

Hi, @scottpetrovic!

To overcome the issue of saving custom properties you can use "method extraction" refactoring pattern. Just define a virtual function that saves/loads some custom assistant information,and call it from the main KisPaintingAssistant::saveXml(). LIke that:

// **********************************************
// in KisPaintingAssistant
// **********************************************
QByteArray KisPaintingAssistant::saveXml(QMap<KisPaintingAssistantHandleSP, int> &handleMap)
{
    QByteArray data;
    QXmlStreamWriter xml(&data);
    xml.writeStartDocument();
    xml.writeStartElement("assistant");
    xml.writeAttribute("type",d->id);
    xml.writeAttribute("active", QString::number(d->isSnappingActive));
    xml.writeStartElement("handles");
    Q_FOREACH (const KisPaintingAssistantHandleSP handle, d->handles) {
        // ...skipped...
    }
    xml.writeEndElement();

    saveCustomXml(xml);

    xml.writeEndElement();
    xml.writeEndDocument();
    return data;
}

// **********************************************
// in VanishingPointAssistant
// **********************************************
virtual void VanishingPointAssistant::saveCustomXml(QXmlStreamWriter &writer) 
{
    xml.writeStartElement("vanishingPointProperties");

    xml.writeStartElement("angleDensity");
    xml.writeAttribute("value", KisDomUtils::toString(realAngleDensityValue));
    xml.writeEndElement();

    xml.writeEndElement();
}

virtual bool VanishingPointAssistant::loadCustomXml(QXmlStreamReader &reader) 
{
    // read stuff
    return true;
}

seeing if I can mark those two comments as "done"