The commit message is too generic, and it does not actually say what was the problem.
Also, the patch is completely unrelated to what the commit message (generically) says.

In D19813#432369, @pino wrote:

The commit message is too generic, and it does not actually say what was the problem.
Also, the patch is completely unrelated to what the commit message (generically) says.

After this patch fff60307ff06 many qCDebug messages make resize slowing down if option transparent is selected.
This commit solves this problem just turning off that debug messages.

In D19813#432376, @andreagenor wrote:

In D19813#432369, @pino wrote:

The commit message is too generic, and it does not actually say what was the problem.
Also, the patch is completely unrelated to what the commit message (generically) says.

After this patch fff60307ff06 many qCDebug messages make resize slowing down if option transparent is selected.
This commit solves this problem just turning off that debug messages.

OK, this solves the "needs explanation" part, and your text is better used as commit message (otherwise this patch seems totally unrelated).

OTOH, fff60307ff06 seems... problematic:

• if enabling all the debug messages slows down the application, I guess we know why they were conditonally inside #ifdef blocks
• I see various blocks like #if DEBUG_KP_COMMAND_HISTORY && 0 that are now unconditionally built... they never were before, even if their define was enabld (because of the && 0); IMHO it would be worth to check whether these blocks are actually in hot paths
• I see some debug blocks still there...
• the initial porting to logging category did not create categories that match the old DEBUG_KP_* defines

@tcanabrava: what about reverting fff60307ff06 instead, since it creates more issues that what actually solves?

Pino, it doesn't. If you enabled the debug messages the code wouldn't
compile as the code that happened with the debug messages where actually
broken.

Now there's a freeze, which is another issue. We can check if the debug is
enabled before trying to output the debug, or remove some of the
problematic debug messages.

Em dom, 17 de mar de 2019 às 08:01, Pino Toscano <
noreply@phabricator.kde.org> escreveu:

pino added a comment. View Revision https://phabricator.kde.org/D19813

In D19813#432376 https://phabricator.kde.org/D19813#432376, @andreagenor
https://phabricator.kde.org/p/andreagenor/ wrote:

In D19813#432369 https://phabricator.kde.org/D19813#432369, @pino
https://phabricator.kde.org/p/pino/ wrote:

The commit message is too generic, and it does not actually say what was
the problem.
Also, the patch is completely unrelated to what the commit message
(generically) says.

After this patch fff60307ff06
https://phabricator.kde.org/R374:fff60307ff06c657a67c81cfbfdb067cbd218880
many qCDebug messages make resize slowing down if option transparent is
selected.
This commit solves this problem just turning off that debug messages.

OK, this solves the "needs explanation" part, and your text is better used
as commit message (otherwise this patch seems totally unrelated).

OTOH, fff60307ff06
https://phabricator.kde.org/R374:fff60307ff06c657a67c81cfbfdb067cbd218880
seems... problematic:

• if enabling all the debug messages slows down the application, I guess we know why they were conditonally inside #ifdef blocks
• I see various blocks like #if DEBUG_KP_COMMAND_HISTORY && 0 that are now unconditionally built... they never were before, even if their define was enabld (because of the && 0); IMHO it would be worth to check whether these blocks are actually in hot paths
• I see some debug blocks still there...
• the initial porting to logging category did not create categories that match the old DEBUG_KP_* defines
  
  @tcanabrava https://phabricator.kde.org/p/tcanabrava/: what about reverting fff60307ff06 https://phabricator.kde.org/R374:fff60307ff06c657a67c81cfbfdb067cbd218880 instead, since it creates more issues that what actually solves?
  
  *REPOSITORY* R374 KolourPaint
  
  *REVISION DETAIL* https://phabricator.kde.org/D19813
  
  *To: *andreagenor, tcanabrava *Cc: *pino, alexeymin, genaxxx

In D19813#432384, @tcanabrava wrote:

Pino, it doesn't. If you enabled the debug messages the code wouldn't
compile as the code that happened with the debug messages where actually
broken.

That is a different issue, which is solved by fixing the debug messages, without the need to unconditionally enable them.

Now there's a freeze, which is another issue. We can check if the debug is
enabled before trying to output the debug, or remove some of the
problematic debug messages.

Or better:

• revert fff60307ff06 in Applications/19.04, so the new stable branch is fixed
• fix the issues I pointed out, namely:

• I see various blocks like #if DEBUG_KP_COMMAND_HISTORY && 0 that are now unconditionally built... they never were before, even if their define was enabld (because of the && 0); IMHO it would be worth to check whether these blocks are actually in hot paths
• I see some debug blocks still there...
• the initial porting to logging category did not create categories that match the old DEBUG_KP_* defines

I don't think is necessary to revert. I'm working to modernize and clean up the code and this commit solves the problem at this time. In the future, I will create a better solution and maybe complete remove these logs.
I think these logs was made just to debug in some development phase but is not required anymore.

In D19813#432402, @andreagenor wrote:

I don't think is necessary to revert. I'm working to modernize and clean up the code and this commit solves the problem at this time. In the future, I will create a better solution and maybe complete remove these logs.

That is fine and nice for the development version.
However, Applications/19.04 has commit fff60307ff06, which means that the upcoming 19.04 version will have these issues, if nothing is done. Even if the freeze starts officially in 4 days, this kind of changes (modernize and refactor) is what IMHO ought to not go to a stable branch.

I still stand on my position about what ought to be done, in Application/19.04 and master.

I'll revert and reply on a dev branch

Em dom, 17 de mar de 2019 às 08:46, Pino Toscano <
noreply@phabricator.kde.org> escreveu:

pino added a comment. View Revision https://phabricator.kde.org/D19813

In D19813#432402 https://phabricator.kde.org/D19813#432402, @andreagenor
https://phabricator.kde.org/p/andreagenor/ wrote:

I don't think is necessary to revert. I'm working to modernize and clean
up the code and this commit solves the problem at this time. In the future,
I will create a better solution and maybe complete remove these logs.

That is fine and nice for the development version.
However, Applications/19.04 has commit fff60307ff06
https://phabricator.kde.org/R374:fff60307ff06c657a67c81cfbfdb067cbd218880,
which means that the upcoming 19.04 version will have these issues, if
nothing is done. Even if the freeze starts officially in 4 days, this kind
of changes (modernize and refactor) is what IMHO ought to not go to a
stable branch.

I still stand on my position about what ought to be done, in
Application/19.04 and master.

*REPOSITORY*
R374 KolourPaint

*REVISION DETAIL*
https://phabricator.kde.org/D19813

*To: *andreagenor, tcanabrava
*Cc: *pino, alexeymin, genaxxx

pushed the revert to both master / applications 19.04