It only supports opacity, size and flow options, there is no configuration
GUI yet, but at least it works and doesn't go crazy every time. This branch
also has a lot of refactoring in KisPaintopBox, which has been awaited for ages.
Details
Details
- Reviewers
rempt - Group Reviewers
Krita - Commits
- R37:5edaa336d499: Merge the Heads-up-display for the brushes into master
Test editing the brush settings with sliders at the top, int the HUD and in
the popup brush manager. The settings should work in a consistent way.
Please also test "Save Tweaks into Presets" and "Locked Settings" features.
Diff Detail
Diff Detail
- Repository
- R37 Krita
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Comment Actions
Dox, dox, dox, of course... And I'm unsure about two things: I don't think I understand the derived resources architecture, that might need some more explanation in the apidox, and I am kinda worried that the erase behaviour might have changed, but when I test, it all seems okay. I'm just not sure I follow the changes correctly!
| libs/flake/KoResourceManager_p.h | ||
|---|---|---|
| 178 ↗ | (On Diff #5502) | s/they/their/ |
| libs/image/brushengine/kis_paintop_settings.h | ||
| 170 ↗ | (On Diff #5502) | This doesn't undo some of nishant's changes, does it? |
| libs/image/brushengine/kis_slider_based_paintop_property.h | ||
| 25 ↗ | (On Diff #5502) | Dox? |
| libs/image/brushengine/kis_standard_uniform_properties_factory.h | ||
| 27 ↗ | (On Diff #5502) | Dox? |
| libs/ui/kis_derived_resources.cpp | ||
| 141 ↗ | (On Diff #5502) | I'm not sure why this class uses this style of comments, but this doesn't line out :-) |
Comment Actions
I don't really understand everything in the code, but going to an MVC type structure seems to be a good idea.
Overall it looks like it is going in a good direction and working as expected. For testing, it seemed to work pretty well overall. I did get a crash on my Ubuntu 16.04 with this workflow...
- Open the HUD and change the brush size on it (leave the pop-up palette open)
- Change brushes in the pop-up palette
This same crash happens with most of the properties if you try them a few times. It is a pretty hard crash, so I didn't get any back trace results or info in the output..
Comment Actions
There is another similar crash in the brush editor, but not sure if it related. I just tested by clicking different brush presets in the brush editor. I tested the pixel engine, but it might be for all of them.
if I change brushes slower like 1-2 seconds between changing, it seems to not crash. If I try to change brush presets more quickly, that seems to crash it.
Comment Actions
14:06 < boud> dmitryK|log1: btw: QLayout: Attempting to add QLayout "" to KisBrushHud "", which already has a layout
Comment Actions
Updated the HUD branch
All the paintops now have the properties, all the phabricator
comments have been resolved. The only thing left to be implemented
is the configuration GUI.
- Implement an Angle HUD property
- Implement Spacing and Auto Spacing uniform properties
- Implement ColorSmudge HUD options
- Implemented uniform properties for Shape and Spray brushes
- Implement HUD properties for hatching brush
- Implement HUD properties for Grid brush
- Implement HUD property for Curve brush
- Implement HUD properties for Dyna brush
- Implement HUD properties for Particle brush
- Added HUD properties to the Clone brush
- Implemented HUD options for the Deform Brush
- Add connection line HUD property to the Curve Brush
- Fix a crash when switching between presets too fast
- Fixed Boud's comments in the review request
- Fix a warning from Qt:
| libs/image/brushengine/kis_paintop_settings.h | ||
|---|---|---|
| 170 ↗ | (On Diff #5502) | No, it just renames the existing methods (and changes semantics a little). |
Comment Actions
Implemented configuration GUI for the Brushes HUD
- Implement configuration GUI for Brush HUD properties editing
- Fix sizing of the Brush HUD
- Added brush icon to the Brush HUD
- Implemented KisElidedLabel class that makes the text elidable