Refactoring to Enable KisPaletteView to Be Used in More Widgets
ClosedPublic

Authored by mzhou on Jun 26 2018, 11:42 PM.

Details

Reviewers
rempt
Group Reviewers
Krita
Summary

KisPaletteView was in kritaui, which depends on kritawidgets. There are also several widgets in kritawidgets that have their own implementation for GUI of a palette. This revision moves KisPaletteView into kritawidgets so that these widgets can use it. This way, in future modification in the palette (or the color set), fewer work would be needed.

The next step is to modify these widgets so that they use KisPaletteView instead of their own implementations.

Test Plan

Relatively heavily modified Widgets:

KisInternalColorSelector
One way to trigger it is to use KoDualColorButton (the buttons in tools bar used to change foreground and background colors or to exchange them)
No modification in the functions of this dialog, so it should work normally.

KisScreenColorPicker
the color picker inside KisDlgInternalColorSelector
No modification in the functions of this widget, so it should work normally.

KisPaintopPresetsPopup
popup shown when clicking the "Edit brush settings" button in the menu bar.
Function that could be broken is repositioning when resizing or when opening the popup when the distance from the button to the right edge of the screen is less than the width of the popup. If broken, part of the popup would be outside of the screen.

KisPaletteView
Widget to show the palette of colors in the palette docker.
Pressing ctrl and rotating the mouse wheel inside in this widget would change the size of swatches.
I changed code related to remembering the size of swatches after resizing. Currently, Krita doesn't remember the size of the swatches, which means the function is broken, but it doesn't seem to work even before I did the modification...

Diff Detail

Repository
R37 Krita
Lint
Lint Skipped
Unit
Unit Tests Skipped
mzhou requested review of this revision.Jun 26 2018, 11:42 PM
mzhou created this revision.
mzhou updated this revision to Diff 36734.Jun 26 2018, 11:45 PM

Removed an unnecessary member in KisPaintopPresetsPopup.
Added a safety guard in its resizeEvent(QResizeEvent *) method

rempt requested changes to this revision.Jun 27 2018, 10:04 AM
rempt added a subscriber: rempt.

On running, I see

QObject::connect: No such signal KisPaletteListWidget::paletteSelected(KoColorSet*)
QObject::connect: (sender name: 'WdgPaletteListWidget')
QObject::connect: (receiver name: 'WdgDlgInternalColorSelector')
QObject::connect: No such signal KisPaletteListWidget::paletteSelected(KoColorSet*)
QObject::connect: (sender name: 'WdgPaletteListWidget')

That needs to be fixed first :-) Also, some inline notes.

libs/pigment/resources/KoColorSetEntry.cpp
5

If you haven't changed anything inside a file, you shouldn't add a copyright line.

libs/pigment/resources/KoColorSetEntry.h
37

I don't think it's a good idea to separate the setters/getters like this.

libs/ui/kis_config.cc
1653–1654

If you removed the implementation, you should also remove the declaration in the header

libs/ui/widgets/KisScreenColorPicker.h
63

Why did you remove the override here?

libs/widgets/KisVisualColorSelectorShape.h
34

I guess you can remover this, since the class isn't exported.

libs/widgets/KisVisualEllipticalSelectorShape.h
34

I guess you can remover this, since the class isn't exported.

libs/widgets/KisVisualRectangleSelectorShape.h
34

I guess you can remover this, since the class isn't exported.

libs/widgets/KisVisualTriangleSelectorShape.h
34

I guess you can remover this, since the class isn't exported.

libs/widgets/kis_palette_view.cpp
88

It's actually not necessary to parent these objects, since adding them to a layout and adding the layout to the dialog will parent them.

This can also lead to messages like:

QLayout: Attempting to add QLayout "" to QWidget "", which already has a layout

303

Same here.

libs/widgets/kis_popup_button.cc
89

Are you sure this isn't needed anymore?

This revision now requires changes to proceed.Jun 27 2018, 10:04 AM
mzhou updated this revision to Diff 36803.Jun 27 2018, 10:52 PM

Removed unnecessary licensing header line
Removed unnecessary inclusion of of export headers
Removed declarations whose implementations are removed
Fixed the override in KisScreenColorPicker
Rearranged the setters and getters in KoColorSetEntry

mzhou updated this revision to Diff 36804.Jun 27 2018, 11:16 PM

Modified resizeEvent(QResizeEvent *) in KisPaintOpPresetsPopup so that it resizes normally when there is no parent

rempt accepted this revision.Jun 29 2018, 7:59 AM

Thanks -- I guess this is fine now :-)

This revision is now accepted and ready to land.Jun 29 2018, 7:59 AM
rempt closed this revision.Jul 1 2018, 7:48 AM