Use Foreground Color for Initial 'New Fill Layer' Dialog
ClosedPublic

Authored by eoinoneill on Dec 12 2018, 12:24 AM.

Details

Reviewers
rempt
dkazakov
woltherav
Group Reviewers
Krita
Summary

When creating a new fill layer within the layer docker ( + Layer > Fill Layer... ) , the new generator layer will start with it's color assigned for whatever the user has as their foreground color.

Workflow Improvement
I believe that this is a better alternative to always starting on black. Users can preemptively select or pick a color to use for their new fill layer.

KisDlgGeneratorLayer Woes
The current design of KisDlgGeneratorLayer makes it difficult to preemptively assign default values and is also prone to some bugs. For instance, going into the properties of a fill layer, switching to pattern, and then back to fill will lose whatever the users settings were when they entered the dialog. KisDlgGeneratorLayer Should probably keep a running configuration setup for every potential generator (initialized from the factory) until the dialog is closed. When closed, it should return the configuration of whichever generator was selected. This would make assigning default values easier and also reduce bug occurrence when poking around within the properties window. As it is right now, getting the "default configuration", changing the color property, and then resetting the configuration seemed to be the easiest way around this issue.

Test Plan

Try making a fill layer with a foreground color. That color should be used as the initial color within the new fill layer dialog.

Diff Detail

Repository
R37 Krita
Lint
Lint Skipped
Unit
Unit Tests Skipped
eoinoneill created this revision.Dec 12 2018, 12:24 AM
Restricted Application added a reviewer: Krita. · View Herald TranscriptDec 12 2018, 12:24 AM
Restricted Application added a project: Krita. · View Herald Transcript
eoinoneill requested review of this revision.Dec 12 2018, 12:24 AM
eoinoneill edited the summary of this revision. (Show Details)Dec 12 2018, 4:39 AM
rempt accepted this revision.Dec 12 2018, 9:05 AM

Yes, makes sense. The only thing I wonder about is -- why the dlg.resize() call?

This revision is now accepted and ready to land.Dec 12 2018, 9:05 AM
eoinoneill added a comment.EditedDec 12 2018, 8:17 PM

I'm actually not sure why the resize call was there in the original code, but I decided to keep it as it was. I moved it closer to dlg.exec() to try to keep window appearance code closer to the window showing code.

eoinoneill closed this revision.Dec 15 2018, 8:33 PM