On 05/02/14 13:59, "Knoll Lars" <[email protected]> wrote:
>Hi John, > >I’ve now gone through all changes. The set looks pretty good overall, and >testing on Linux showed no regressions compared to 5.2. One correction here: The paperRect as shown in the preview of the dialogs manual test shows some slightly different results, that look like a rounding issue. Not sure whether that’s a problem that should be fixed or not, but you should have a look. > >IMO, you can go ahead and stage the first set of changes that are mainly >cleanups. > >There are quite a few comments on the changes adding the new public API. >Nothing major, but it would be good if you could go through the series and >fix patches according to the comments. There’s also a compile error with >QPageMargins that somehow is not catched on Linux (no idea why not). > >Friedemann said that there were still issues with printing on Windows (see >comments on https://codereview.qt-project.org/#change,76913). > >Could you have a look at these things and update the patch set? > >Thanks! >Lars > > >On 30/01/14 09:35, "Sorvig Morten" <[email protected]> wrote: > >>On 30 Jan 2014, at 01:15, John Layt <[email protected]> wrote: >>> >>> I've just pushed a 33 commit change set for this to Gerrit, my >>>apologies to >>> the people I've tagged as reviewers :-) Any one else interested, feel >>>free to >>> jump in and help. >>> >>> There's 4 main new classes: >>> * QPageSize >>> * QPageMargins >>> * QPageLayout >>> * QPlatformPrintDevice, with backends for Windows, Mac and Cups >>> >>> These are then used in a fairly major rewrite of the platform and PDF >>>print >>> engines and QPrinter itself. I've tested these changes using my >>>limited set >>> of printers and some test painting code, but obviously I can't test for >>> everything, especially the wide variety of drivers and the dodgy data >>>they may >>> return. I'd appreciate if people could check out the last commit in >>>the chain >>> [1] and see if it works OK with your printers and apps and report any >>>problems >>> back here. >> >>Thanks for the contribution! My initial impression is that it looks like >>a solid design and implementaiton. I’m not sure how to review it in >>detail - you are now the expert on Mac printing. >> >>We know from our work with the new platform plugins for Qt 5 that a >>re-implementation of existing functionality is unlikely to be bug-for-bug >>compatible. I can’t really say what the state is for printing, but I >>would like to avoid a situation where 5.3 improves in some areas but >>regresses in others. >> >>Have you considered keeping the current implementation side-by-side with >>the new one and adding an opt-in/opt-out mechanism for 5.3? This could >>then be accompanied with a blog post or other suitably public >>announcement that *now* is your chance to test and report bugs against >>the new printing implementation before the old one is removed. >> >>As a reviewer I will at least be more comfortable giving a +2 if I know >>that the new code will be field tested before becoming the only print >>support in Qt. >> >>Morten >>_______________________________________________ >>Development mailing list >>[email protected] >>http://lists.qt-project.org/mailman/listinfo/development > >_______________________________________________ >Development mailing list >[email protected] >http://lists.qt-project.org/mailman/listinfo/development _______________________________________________ Development mailing list [email protected] http://lists.qt-project.org/mailman/listinfo/development
