[Geany-Devel] Printing

[Colomban Wendling]

Hi,

Le 31/10/2012 06:58, Lex Trotman a écrit :
> Hi Roger,
> 
> A couple of comments.
> 
> 1. It is unlikely that the old non-Scintilla printing code will be
> returned to Geany.  Sorry that you made changes to it before you were
> aware of the change to Scintilla printing.  It would be definitely
> best to keep up to date with the current Git if you are making
> changes, since they will need to be merged into it.

I agree with Lex here.  The old code was extremely complex and tried to
mimic Scintilla display, while failing to deal with many things
(counting wrapped lines, paginating the output, and probably others I
don't remember).

I think that if you want to give the printing some love, you should
rather work together with Scintilla to get it do what you (we) want.
For example, Scintilla could have an API to chose between whitespace
between line numbers and code, or the line we want.

> 2. Those problems that you found can be reproduced on Scite should be
> reported to the Scintilla project. (www.scintilla.org)
> 
> Cheers
> Lex
> 
> On 31 October 2012 16:36, Roger Booth <rbooth at kabooth.com> wrote:
>> I have been investigating Geany's problems with printing.
>>
>> I looked at
>>  * Geany 1.22
>>  * Geany - the current git (as of a few days ago when I started this)
>>  * Geany 1.23 (git >= 8855c14), en_US.UTF-8
>>  * SciTE 3.1.0
>>
>>
>> Geany 1.22 - problems
>>  * When counting pages, tabs are not expanded, giving inaccurate results -
>> #3475444
>>  * Printing code mishandles non-monospaced fonts - #2804000
>>  * When printing, tabs are expanded to fixed width, not to the next tab stop
>> - #2629121.
>>  * When user requests to print a specified page, page 1 is printed and
>> labeled the requested page.
>> For example, if the user selects to print page 7 only, Geany will print
>> page 1, but in the heading, the text will read “Page 7 of xx”. I didn't
>> submit this as a developer bug, since it is fixed in the current version.

Those issues are the reason I spend some effort switching to the
Scintilla printing support.  Especially the pagination issue was quite
hard to fix because the layout and display code were merged together, so
it was nearly impossible to just layout without actually render (and we
need that to layout the pages that won't be printed in order to get the
line offset at which start to print).

Moreover, the old code had sever issues when wrapping the last line on
the page.

>> Geany – the current git - problems
>>  * Print Preview displays a different number of lines than hard-copy -
>> #3580269.
>>  * The line separating line numbers from text when printing is positioned
>> incorrectly - #3580268.

I'm not sure, but I suspect this might be because of the "magnification"
setting from scintilla.  You noted I added a quite clear comment next to
that call, showing I have roughly no idea why -2 gives good results.  To
get this right, either the fixes has to go in Scintilla, or we need to
understand exactly what that magnification is, so what to reproduce.

>>  * Printed lines are right-shifted compared to previous Geany versions,
>> causing wrapping which did not occur in previous Geany versions.

As you noted, it's Scintilla that adds whitespace to separate line
numbers from code, but we prefer a vertical line.

>> SciTE problems
>>  * Print Preview displays a different number of lines than hard-copy.

So this one is not specific to our printing code, and both Geany and
SciTE would benefit from a fix.  Or maybe it's just I took some SciTE
code as an example, e.g. for the setup_range() function.

Unfortunately I don't currently have a real printer to test with, so I
only tested with PDF and preview output;  I didn't suspect that real
copy could look different.

>> The git version of Geany contains a new version of the printing code. This
>> new version uses the printing code contained within Scintilla.
>>
>> The Scintilla printing code does not draw a vertical line between the line
>> number and the text. So the git version of Geany attempts to draw that line.
>> However, Scintilla provides no method of determining the width of the line
>> number column (SSM(SCI_GETMARGINWIDTHN) returns the display margin width,
>> not the printer margin width). So the Geany code attempts to mimic the
>> calculations Scintilla performs to come up with the column width. The
>> attempt fails, thus bug #3580268.

I know.  I first tried, as you suggest, to use GETMARGINWITH, but as you
saw, without any success unfortunately.

>> The Scintilla printing code adds three spaces between line number and text.
>> This shifts the text right, and lines which were nearly page width when
>> printed using the previous versions of Geany now wrap.

Yes, and I agree this is unwanted since we have our less space-wasting
vertical line separator.

>> As a quick check whether the lines per page mismatch between Print Preview
>> and hard copy is in Geany's code or Scintilla's, I tested SciTE. SciTE has
>> the same problem.
>>
>> Using the git version of Geany, I printed page 1 of a particular test
>> document, Print Preview listed the document as 18 pages long with 51 lines
>> on the first page (4 lines wrapped) and the hardcopy listed it as 19 pages
>> long with 45 lines on the first page (8 lines wrapped).
>>
>> Printing the same test with SciTE, Print Preview listed 55 lines on page 1
>> with 3 lines wrapped, and hardcopy listed 44 lines with 7 lines wrapped.
>> (SciTE's output does not show the number of pages in the document).

That difference of lines between Geany and SciTE is probably due to our
header and footer I guess.

>> I have investigated the problems with the old (1.22 and before) printer
>> code, and I believe all can be addressed by modifying the original code,
>> with the exception of
>> Printing code mishandles non-monospaced fonts - #2804000
>>
>>
>> I fixed the bug “When printing, tabs are expanded to fixed width, not to the
>> next tab stop - #2629121” locally a few weeks ago. That was why I joined the
>> developers group. I wanted to submit that patch.
>>
>> I have not noticed any discrepancy between Print Preview and hardcopy with
>> the (tab expansion patched) old code. And the old code naturally knows the
>> correct location for the vertical line.
>>
>> When determining the number of pages in the code, the old code currently
>> determines the length of each line by multiplying the width of one character
>> by the number of characters in the line. That is correct as far as UTF8 is
>> concerned – it uses a UTF-aware function to determine number of characters.
>> But it fails to take into consideration the expansion of tabs.
>>
>> To fix this problem in the old code, one could replace the page counting
>> code. Take the lines of code that actually do the printing, put those lines
>> into a separate function, and call that function twice. First time, the
>> function would calculate the layout (and thus the number of pages) but not
>> actually print. Second time, it prints. If only certain pages are requested,
>> second time it skips lines til a requested page's first line, then prints
>> the page, then skips lines til the next requested page, etc.

That'd be a bunch of work, and beware: the code must deal with wrapped
lines spanning between pages.  E.g. if last line of page 1 wraps, its
end must appears on page 2, no matter whether page 1 was actually
printed or not.

>> Once tabs are expanded correctly, this change seems to fix all the issues
>> except non-monospaced fonts.
>>
>> An option to deal with that could be to use the new code only for
>> non-monospaced fonts. For monospaced fonts, use the old code. For
>> non-monospaced fonts, the issue of Print Preview not matching hardcopy would
>> remain, but IMO that is a Scintilla bug, and depends on that group for a
>> fix.

I don't like the old code since it isn't really clever and tries to
replicates something that is already there and better done: Scintilla.
And I like even less the idea of having two code paths for one thing.

So, I'm sorry you spent some effort on the old, now removed, code but I
don't think bringing it back is the solution.  I think the new code
should rather be fixed, and Scintilla too if that's needed.


Best regards,
Colomban