From 3d5810f0887fcccced2b219eaebf14259a7ba18c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Batty=C3=A1nyi?= <86350313+LoKolbasz@users.noreply.github.com> Date: Sat, 19 Aug 2023 00:17:10 +0200 Subject: [PATCH] Applied requested changes from review comments --- rtgui/filmnegative.cc | 44 +++++++++++-------------------------------- rtgui/filmnegative.h | 19 ++----------------- rtgui/guiutils.cc | 33 +++++++++++++------------------- rtgui/guiutils.h | 22 +++++++++++++++++----- 4 files changed, 43 insertions(+), 75 deletions(-) diff --git a/rtgui/filmnegative.cc b/rtgui/filmnegative.cc index f21ed8d9f..66002e23c 100644 --- a/rtgui/filmnegative.cc +++ b/rtgui/filmnegative.cc @@ -212,7 +212,7 @@ FilmNegative::FilmNegative() : picker(DEFAULT_SPOT_WIDTH, M("TP_FILMNEGATIVE_PICK"), M("TP_FILMNEGATIVE_GUESS_TOOLTIP"), M("TP_FILMNEGATIVE_PICK_SIZE")), refInputLabel(Gtk::manage(new Gtk::Label(Glib::ustring::compose(M("TP_FILMNEGATIVE_REF_LABEL"), "- - -")))), refPicker(DEFAULT_SPOT_WIDTH, M("TP_FILMNEGATIVE_REF_PICK"), M("TP_FILMNEGATIVE_REF_TOOLTIP"), M("TP_FILMNEGATIVE_REF_SIZE")), - displayRectWidth(&(picker._associatedVar)), + activePicker(&picker), outputLevel(createLevelAdjuster(this, M("TP_FILMNEGATIVE_OUT_LEVEL"))), // ref level greenBalance(createBalanceAdjuster(this, M("TP_FILMNEGATIVE_GREENBALANCE"), -3.0, 3.0, 0.0, "circle-magenta-small.png", "circle-green-small.png")), // green balance blueBalance(createBalanceAdjuster(this, M("TP_FILMNEGATIVE_BLUEBALANCE"), -3.0, 3.0, 0.0, "circle-blue-small.png", "circle-yellow-small.png")) // blue balance @@ -242,9 +242,6 @@ FilmNegative::FilmNegative() : pack_start(*redRatio, Gtk::PACK_SHRINK, 0); pack_start(*blueRatio, Gtk::PACK_SHRINK, 0); pack_start(picker, Gtk::PACK_SHRINK, 0); - // pack_start(*spotButton, Gtk::PACK_SHRINK, 0); - - // pack_start(*oldMethod, Gtk::PACK_SHRINK, 0); Gtk::Separator* const sep = Gtk::manage(new Gtk::Separator(Gtk::ORIENTATION_HORIZONTAL)); sep->get_style_context()->add_class("grid-row-separator"); @@ -262,9 +259,8 @@ FilmNegative::FilmNegative() : pack_start(refPicker, Gtk::PACK_SHRINK, 0); - picker._spotButton.signal_toggled().connect(sigc::mem_fun(*this, &FilmNegative::editToggled)); - - refPicker._spotButton.signal_toggled().connect(sigc::mem_fun(*this, &FilmNegative::refSpotToggled)); + picker.add_button_toggled_event(*this, &FilmNegative::editToggled); + refPicker.add_button_toggled_event(*this, &FilmNegative::refSpotToggled); // Editing geometry; create the spot rectangle // TODO: Change behaviour to match that of the white balance spot picker (rectangle disappears behind right toolbar) @@ -432,8 +428,8 @@ void FilmNegative::setBatchMode(bool batchMode) ToolPanel::setBatchMode(batchMode); if (batchMode) { - removeIfThere(this, &picker._spotButton, false); - removeIfThere(this, &refPicker._spotButton, false); + picker.remove_if_there(this, false); + refPicker.remove_if_there(this, false); colorSpace->append(M("GENERAL_UNCHANGED")); colorSpace->set_active_text(M("GENERAL_UNCHANGED")); redRatio->showEditedCB(); @@ -542,7 +538,7 @@ bool FilmNegative::mouseOver(int modifierKey) { EditDataProvider* const provider = getEditProvider(); Rectangle* const spotRect = static_cast(visibleGeometry.at(0)); - spotRect->setXYWH(provider->posImage.x - *displayRectWidth, provider->posImage.y - *displayRectWidth, *displayRectWidth * 2, *displayRectWidth * 2); + spotRect->setXYWH(provider->posImage.x - activePicker->get_spot_half_width(), provider->posImage.y - activePicker->get_spot_half_width() ,activePicker->get_spot_half_width() * 2, activePicker->get_spot_half_width() * 2); return true; } @@ -564,8 +560,8 @@ bool FilmNegative::button1Pressed(int modifierKey) RGB ref1, ref2, dummy; - if (fnp->getFilmNegativeSpot(refSpotCoords[0], picker._associatedVar, ref1, dummy) && - fnp->getFilmNegativeSpot(refSpotCoords[1], picker._associatedVar, ref2, dummy)) { + if (fnp->getFilmNegativeSpot(refSpotCoords[0], picker.get_spot_half_width(), ref1, dummy) && + fnp->getFilmNegativeSpot(refSpotCoords[1], picker.get_spot_half_width(), ref2, dummy)) { disableListener(); @@ -611,7 +607,7 @@ bool FilmNegative::button1Pressed(int modifierKey) } RGB refOut; - fnp->getFilmNegativeSpot(provider->posImage, refPicker._associatedVar, refInputValues, refOut); + fnp->getFilmNegativeSpot(provider->posImage, refPicker.get_spot_half_width(), refInputValues, refOut); // Output luminance of the sampled spot float spotLum = rtengine::Color::rgbLuminance(refOut.r, refOut.g, refOut.b); @@ -685,9 +681,7 @@ void FilmNegative::editToggled() refPicker.set_active(false); refSpotCoords.clear(); - displayRectWidth = &(picker._associatedVar); - // if (spotlistener) - // spotlistener->spotNegRequested(spotWidth); + activePicker = &picker; subscribe(); @@ -711,9 +705,7 @@ void FilmNegative::refSpotToggled() picker.set_active(false); refSpotCoords.clear(); - displayRectWidth = &(refPicker._associatedVar); - // if (spotlistener) - // spotlistener->spotNegRequested(refSpotWidth); + activePicker = &refPicker; subscribe(); @@ -731,17 +723,3 @@ void FilmNegative::refSpotToggled() } } -// void FilmNegative::spotSizeChanged () -// { -// spotWidth = atoi(spotSize->get_active_text().c_str()); - -// // if (spotlistener) -// // spotlistener->spotNegRequested(spotWidth); -// } - -// void FilmNegative::refSpotChanged() -// { -// refSpotWidth = atoi(refSpotSize->get_active_text().c_str()); -// // if (spotlistener) -// // spotlistener->spotNegRequested(refSpotWidth); -// } diff --git a/rtgui/filmnegative.h b/rtgui/filmnegative.h index 02d032f3c..87e91af8c 100644 --- a/rtgui/filmnegative.h +++ b/rtgui/filmnegative.h @@ -43,12 +43,6 @@ public: virtual bool getFilmNegativeSpot(rtengine::Coord spot, int spotSize, RGB &refInput, RGB &refOutput) = 0; }; -// class FilmNegSpotListener -// { -// public: -// virtual ~FilmNegSpotListener() = default; -// virtual void spotNegRequested(int size) = 0; -// }; class FilmNegative final : public ToolParamBlock, @@ -85,10 +79,6 @@ public: bool button1Released() override; bool button3Pressed(int modifierKey) override; void switchOffEditMode() override; - // void setFilmNegSpotListener(FilmNegSpotListener* listener) - // { - // spotlistener = listener; - // } private: void editToggled(); @@ -97,9 +87,6 @@ private: void readOutputSliders(RGB &refOutput); void writeOutputSliders(const RGB &refOutput); - // void spotSizeChanged(); - // void refSpotChanged(); - // ColorTemp value corresponding to neutral RGB multipliers (1,1,1). Should be around 6500K. const rtengine::ColorTemp NEUTRAL_TEMP; @@ -109,8 +96,6 @@ private: const rtengine::ProcEvent evFilmNegativeBalance; const rtengine::ProcEvent evFilmNegativeColorSpace; - // FilmNegSpotListener* spotlistener; - std::vector refSpotCoords; RGB refInputValues; @@ -135,14 +120,14 @@ private: Adjuster* const redRatio; Adjuster* const blueRatio; - #define DEFAULT_SPOT_WIDTH 8 + static constexpr int DEFAULT_SPOT_WIDTH = 8; SpotPicker picker; Gtk::Label* const refInputLabel; SpotPicker refPicker; - int* displayRectWidth; + SpotPicker* activePicker; Adjuster* const outputLevel; Adjuster* const greenBalance; diff --git a/rtgui/guiutils.cc b/rtgui/guiutils.cc index e3e52fc23..7d65b9e46 100644 --- a/rtgui/guiutils.cc +++ b/rtgui/guiutils.cc @@ -1926,32 +1926,25 @@ void BackBuffer::copySurface(Cairo::RefPtr crDest, Gdk::Rectangl SpotPicker::SpotPicker(int const defaultValue, Glib::ustring const &buttonKey, Glib::ustring const &buttonTooltip, Glib::ustring const &labelKey) : Gtk::Grid(), - _associatedVar(defaultValue), + _spotHalfWidth(defaultValue), _spotLabel(labelSetup(labelKey)), - _spotSizeSetter(new MyComboBoxText(selecterSetup())), + _spotSizeSetter(MyComboBoxText(selecterSetup())), _spotButton(spotButtonTemplate(buttonKey, buttonTooltip)) { - Gtk::Grid* spotSizeHelper = new Gtk::Grid(); - spotSizeHelper->set_name("Spot-Size-Helper"); - setExpandAlignProperties(spotSizeHelper, false, false, Gtk::ALIGN_START, Gtk::ALIGN_CENTER); - this->get_style_context()->add_class("grid-spacing"); setExpandAlignProperties(this, true, false, Gtk::ALIGN_FILL, Gtk::ALIGN_CENTER); - spotSizeHelper->attach (*_spotSizeSetter, 0, 0, 1, 1); - this->attach (_spotButton, 0, 0, 1, 1); this->attach (_spotLabel, 1, 0, 1, 1); - this->attach (*spotSizeHelper, 2, 0, 1, 1); - _spotSizeSetter->signal_changed().connect( sigc::mem_fun(*this, &SpotPicker::spotSizeChanged)); + this->attach (_spotSizeSetter, 2, 0, 1, 1); + _spotSizeSetter.signal_changed().connect( sigc::mem_fun(*this, &SpotPicker::spotSizeChanged)); } SpotPicker::~SpotPicker() { - delete _spotSizeSetter; } -Gtk::Label SpotPicker::labelSetup(Glib::ustring const &key) +Gtk::Label SpotPicker::labelSetup(Glib::ustring const &key) const { Gtk::Label label(key); setExpandAlignProperties(&label, false, false, Gtk::ALIGN_START, Gtk::ALIGN_CENTER); @@ -1961,34 +1954,34 @@ Gtk::Label SpotPicker::labelSetup(Glib::ustring const &key) MyComboBoxText SpotPicker::selecterSetup() { MyComboBoxText spotSize = MyComboBoxText(); - setExpandAlignProperties(&spotSize, true, false, Gtk::ALIGN_FILL, Gtk::ALIGN_CENTER); + setExpandAlignProperties(&spotSize, false, false, Gtk::ALIGN_START, Gtk::ALIGN_CENTER); spotSize.append ("2"); - if (_associatedVar == 2) { + if (_spotHalfWidth == 2) { spotSize.set_active(0); } spotSize.append ("4"); - if (_associatedVar == 4) { + if (_spotHalfWidth == 4) { spotSize.set_active(1); } spotSize.append ("8"); - if (_associatedVar == 8) { + if (_spotHalfWidth == 8) { spotSize.set_active(2); } spotSize.append ("16"); - if (_associatedVar == 16) { + if (_spotHalfWidth == 16) { spotSize.set_active(3); } spotSize.append ("32"); - if (_associatedVar == 32) { + if (_spotHalfWidth == 32) { spotSize.set_active(4); } return spotSize; @@ -2006,5 +1999,5 @@ Gtk::ToggleButton SpotPicker::spotButtonTemplate(Glib::ustring const &key, const void SpotPicker::spotSizeChanged() { - _associatedVar = atoi(_spotSizeSetter->get_active_text().c_str()); -} \ No newline at end of file + _spotHalfWidth = atoi(_spotSizeSetter.get_active_text().c_str()); +} diff --git a/rtgui/guiutils.h b/rtgui/guiutils.h index 77cbdbd93..48ccb5e60 100644 --- a/rtgui/guiutils.h +++ b/rtgui/guiutils.h @@ -694,12 +694,12 @@ public: */ class SpotPicker : public Gtk::Grid { - public: - int _associatedVar; + private: + int _spotHalfWidth; Gtk::Label _spotLabel; - MyComboBoxText* const _spotSizeSetter; + MyComboBoxText _spotSizeSetter; Gtk::ToggleButton _spotButton; - + public: SpotPicker(int const defaultValue, Glib::ustring const &buttonKey, Glib::ustring const &buttonTooltip, Glib::ustring const &labelKey); ~SpotPicker(); inline bool get_active() @@ -710,9 +710,21 @@ class SpotPicker : public Gtk::Grid { _spotButton.set_active(b); } + int get_spot_half_width() + { + return _spotHalfWidth; + } + template void add_button_toggled_event(T_return& returnv, const T_obj function) + { + _spotButton.signal_toggled().connect(sigc::mem_fun(returnv, function)); + } + bool remove_if_there(Gtk::Container* cont, bool increference = true) + { + return removeIfThere(cont, &_spotButton, increference); + } protected: - static Gtk::Label labelSetup(Glib::ustring const &key); + Gtk::Label labelSetup(Glib::ustring const &key) const; MyComboBoxText selecterSetup(); static Gtk::ToggleButton spotButtonTemplate(Glib::ustring const &key, const Glib::ustring &tooltip); void spotSizeChanged();