Bug 421084

Summary: t variable is reasigned in kis_color_selector.cpp
Product: [Applications] krita Reporter: Rafał Mikrut <mikrutrafal>
Component: GeneralAssignee: Tiar <tamtamy.tymona>
Status: RESOLVED FIXED    
Severity: normal CC: alvin, bourumir.wyngs, scottpetrovic, tamtamy.tymona
Priority: NOR    
Version First Reported In: git master (please specify the git hash!)   
Target Milestone: ---   
Platform: Compiled Sources   
OS: Linux   
Latest Commit: Version Fixed/Implemented In:
Sentry Crash Report:

Description Rafał Mikrut 2020-05-05 20:44:15 UTC
```

        qreal t = (pt.x() - m_lightStripArea.x()) / qreal(m_lightStripArea.width());
        t = (pt.y() - m_lightStripArea.y()) / qreal(m_lightStripArea.height());

```

https://invent.kde.org/kde/krita/-/blob/master/plugins/dockers/artisticcolorselector/kis_color_selector.cpp#L348-349
Comment 1 Scott Petrovic 2020-05-05 20:47:22 UTC
Usually for code changes, there are not done in bugzilla. We have a different process for changing code through our GitLab instance. Usually for bugzilla, the important thing is having reprodicible steps to get a bug. These seem more like code refactorings. 

https://invent.kde.org/kde/krita

It looks like you have already created a fork. Could you just make the fixes and do Pull requests for these fixes?

https://invent.kde.org/qarmin/krita

That would help us out a lot.
Comment 2 Tiar 2021-08-19 20:04:47 UTC
It's more like this: https://invent.kde.org/graphics/krita/-/blob/589911494cce6af4c49180fbfcee959d190eaf1a/plugins/dockers/artisticcolorselector/kis_color_selector.cpp#L336-339

The first line probably doesn't matter, since the important part is only the y/height part.
Comment 3 Bourumir Wyngs 2022-04-11 17:50:03 UTC
I think this is not "for code change" but rather there are serious reasons to suspect that the code does a different thing than it was intended by the original author.
Comment 4 Alvin Wong 2022-04-12 08:58:17 UTC
From the change 7 years ago:

https://invent.kde.org/graphics/krita/-/commit/fa81254fddf0174efd078826f66e471dd36caf23#c430ca98d9313104ab986c663faf7dc19b48b615_189_177

I would assume that the initial assignment to t is indeed no longer needed, and is just leftover code.
Comment 5 Halla Rempt 2022-08-11 10:45:11 UTC
Git commit ee0adef666d1d0296173fd0be5be9055968cccb2 by Halla Rempt.
Committed on 11/08/2022 at 10:44.
Pushed by rempt into branch 'master'.

Remove obsolete code

M  +1    -3    plugins/dockers/artisticcolorselector/kis_color_selector.cpp

https://invent.kde.org/graphics/krita/commit/ee0adef666d1d0296173fd0be5be9055968cccb2
Comment 6 Halla Rempt 2022-08-11 10:45:33 UTC
Git commit 92324e903cb6c51631db4b1111b0dd733b146ba5 by Halla Rempt.
Committed on 11/08/2022 at 10:45.
Pushed by rempt into branch 'krita/5.1'.

Remove obsolete code
(cherry picked from commit ee0adef666d1d0296173fd0be5be9055968cccb2)

M  +1    -3    plugins/dockers/artisticcolorselector/kis_color_selector.cpp

https://invent.kde.org/graphics/krita/commit/92324e903cb6c51631db4b1111b0dd733b146ba5