View Issue Details

IDProjectCategoryView StatusLast Update
0012650ScribusGeneralpublic2015-12-01 23:01
Reporterwilliam Assigned To 
PrioritynormalSeverityminorReproducibilityN/A
Status newResolutionopen 
Platformx86_64OSLinuxOS VersionFedora 20
Summary0012650: [patch] Make debug code in pageitem_textframe.cpp easier to enable.
DescriptionWhen I looked at layout differences between Scribus 1.4 and 1.5 in http://bugs.scribus.net/view.php?id=12636 , I saw a lot of qDebug() debug code in pageitem_textframe.cpp that was commented out.
These patches switch those qDebug() calls to scDebug() so that the debug code can be enabled by setting SC_DEBUG_FILE inside pageitem_textframe.cpp or DEBUG_PAGEITEMTEXTFRAME during the compile instead of adding or removing a lot of comments.
The patches also fix some of the commented debug code that no longer compiled.

I also extended scdebug.h.
* Modules that include scdebug.h can now set the macro SC_DEBUG_MODULE to make unique names for the classes that it defines. Without this change, if some modules enable scdebug and other modules disable it, scribus will crash.
* Modules that include scdebug.h can now set the macro SC_DEBUG_LOG_TIME to 0 to suppress adding a time stamp. Suppressing the time stamp makes it easier to compare logs from different runs.

If a small change to an SLA or to pageitem_textframe causes unexpected differences in the layout, this patch makes it fast to enable debugging and compare logs from before and after the change to identify the point where the layout goes wrong.

These patches should not affect scribus functionality.
TagsNo tags attached.
PatchYes

Activities

william

2014-09-02 20:06

updater  

Kunda

2014-10-07 22:13

updater   ~0033921

Moving out of Contributor Builds. William, please post patches to main Scribus queue and not Contributor Build. Core devs will see it better.

william

2014-10-08 00:44

updater   ~0033927

How do I post to the main Scribus queue?
Should I have entered a different value in one of the fields?

cbradney

2014-10-08 06:07

administrator   ~0033929

Project - Scribus

Kunda

2014-10-15 14:12

updater   ~0034014

Making this 'Sticky' so it doesn't suffer "Patch Rot"

jghali

2014-10-15 19:13

administrator   ~0034019

Last edited: 2014-10-15 19:17

Hmmm, unconditional debug code in text layout such as below is a no go for me:
scDebug() << QString("dropcaps 1pre: chsd=%1 realCharHeight = %2 chstr=%3").arg(chsd).arg(asce).arg(chstr[0]);
The arg() function will be executed unconditionally here.

jghali

2014-10-15 19:19

administrator   ~0034020

Unsticking: as is, this patch cannot be applied.

william

2014-10-15 20:21

updater   ~0034021

Would it be acceptable if I put all of the debug inside "#ifdef DEBUG_PAGEITEMTEXTFRAME"?

ale

2014-10-16 12:07

manager   ~0034024

william, if you put a .diff extension to your patches (or maybe even .patch) they can be read directly in the bug tracker...

ale

2014-10-16 12:15

manager   ~0034025

on the one side, i agree that it looks like a good idea to be able to enable / disable a set of debug instances with a single switch...

... but my experience shows that as soon as you start using this regularly, i get too many error messages, most of them set by other developers...

while i admit that i also like qDebug(), i would say that using the debugger (in qt creator or through gdb) is probably a better choice for tracking the behavior of the code...

keep up with the stream of patches!

ciao
a.l.e

Kunda

2014-10-21 23:46

updater   ~0034113

verdict?

william

2014-10-22 00:15

updater   ~0034115

pageitem_textframe.cpp is one of the few Scribus modules with a lot of debug code, and I think that it is easier to enable and disable debug code if you can set a single #if at the top than if you have to add and remove comments in a lot of places.

I started looking there because I had documents made by Scribus 1.4 that formatted differently in Scribus 1.5 but apparently it was bugs in Scribus 1.4.

It does not matter to me whether these patches are applied or not, but if there are changes that would make the patches acceptable, I can do that.

Also, if you want, I can separate the patches to scdebug.h that allow it to be used conditionally with multiple modules. Those patches might be useful on their own separately from the patches to pageitem_textframe.cpp.

Kunda

2014-11-04 17:08

updater   ~0034188

bump

Kunda

2015-01-10 17:37

updater   ~0034362

would be good to make a decision on this before patch becomes too outdated

Kunda

2015-10-07 20:57

updater   ~0036541

Craig & Jean, would this help, for example to not have to compile scribus as much since the debug output would be helpful enough to understand runtime issues ? Anytime we can save an opportunity to build scribus is a good timesaver, right? or am I way off?

ale

2015-10-12 09:04

manager   ~0036648

being able to output debug information that is relevant to the user to better understand bugs is a good idea.

but i would avoid to do so by default. do we need a command line switch for it?

ad noted above, for programmers, using the real debugger is probably a better choice than using "qDebug() <<" to output to the terminal...
i know, it's sometime faster to use the standard output, but i would not implement a way to enable / disable the qDebugs() for such a use case.

JLuc

2015-10-12 15:25

developer   ~0036653

I've worked with #ifdef DEBUG_MODE_XXX and it prooved very efficient.

Another powerfull debug tool is extensive use of user defined asserts including high level asserts.

Both of these tools dont require masses of work to be efficient : the changes can be done localy in the code where-when needed, step by step.

ale

2015-11-03 10:57

manager   ~0037174

asserts would be very welcome!

Kunda

2015-12-01 22:32

updater   ~0037779

http://doc.qt.io/qt-5/qloggingcategory.html <--possibly another way to achieve this ?

william

2015-12-01 23:01

updater   ~0037781

The code already uses QDebug. Scribus has an scDebug layer over QDebug that allows the generation of debug code to be controlled by a #define at compile time so that production code doesn't have to go to the expense of preparing the lines to print when debugging is not enabled.
I think that people on the CTL project have been working on pageitem_textframe.cpp, so the part of the patches that adds debug code to it is probably no longer relevant.
The other part of the patches that enhances scDebug might still be useful.

Issue History

Date Modified Username Field Change
2014-09-02 20:06 william New Issue
2014-09-02 20:06 william File Added: scribus15-pageitem-textframe-debug.pat
2014-09-23 12:57 JLuc Tag Attached: #layout
2014-10-07 22:13 Kunda Note Added: 0033921
2014-10-07 22:13 Kunda Project Contributor Builds => Scribus
2014-10-08 00:44 william Note Added: 0033927
2014-10-08 06:07 cbradney Note Added: 0033929
2014-10-08 19:57 JLuc File Added: project - scribus.jpg
2014-10-13 15:53 Kunda Tag Detached: #layout
2014-10-15 14:12 Kunda Sticky Issue No => Yes
2014-10-15 14:12 Kunda Note Added: 0034014
2014-10-15 19:13 jghali Note Added: 0034019
2014-10-15 19:17 jghali Note Edited: 0034019
2014-10-15 19:19 jghali Sticky Issue Yes => No
2014-10-15 19:19 jghali Note Added: 0034020
2014-10-15 20:21 william Note Added: 0034021
2014-10-16 12:07 ale Note Added: 0034024
2014-10-16 12:15 ale Note Added: 0034025
2014-10-21 23:46 Kunda Note Added: 0034113
2014-10-22 00:15 william Note Added: 0034115
2014-10-24 22:57 Kunda Patch => Yes
2014-10-29 01:37 Kunda Sticky Issue No => Yes
2014-11-04 17:08 Kunda Note Added: 0034188
2014-11-04 17:09 Kunda File Deleted: project - scribus.jpg
2015-01-10 17:37 Kunda Note Added: 0034362
2015-10-07 20:57 Kunda Note Added: 0036541
2015-10-12 09:04 ale Note Added: 0036648
2015-10-12 15:25 JLuc Note Added: 0036653
2015-11-02 18:47 aliyousuf Tag Attached: HOST-Oman
2015-11-03 10:06 user5122 Tag Detached: HOST-Oman
2015-11-03 10:57 ale Note Added: 0037174
2015-11-05 12:18 Kunda Sticky Issue Yes => No
2015-12-01 22:32 Kunda Note Added: 0037779
2015-12-01 23:01 william Note Added: 0037781