| Summary: | Printing test results differ between machines, we should use ImageDiff instead | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Eric Seidel (no email) <eric> | ||||
| Component: | Tools / Tests | Assignee: | Nobody <webkit-unassigned> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | abarth, cjerdonek, darin, dglazkov, dpranke, eric, hamaji, webkit.review.bot | ||||
| Priority: | P2 | ||||||
| Version: | 528+ (Nightly build) | ||||||
| Hardware: | Mac | ||||||
| OS: | OS X 10.5 | ||||||
| Bug Depends on: | 37203 | ||||||
| Bug Blocks: | |||||||
| Attachments: |
|
||||||
|
Description
Eric Seidel (no email)
2008-07-11 14:42:06 PDT
Hi Eric, (of course, comments from other people are also welcomed! :) Though I added two functions into layoutTestController so we can test some of paged media related stuff, we still think we eventually need a way to dump whole pages as a image. For example, even if we implement margin boxes, our current test harness provides no way to test it. So, I'm thinking about the best way to provide a way to dump all pages. I'm not sure if dumping PDFs is the best as it would require some amount of efforts to make it portable among ports. I'm considering to create printToPNG, which would dump all pages into a PNG. With this approach, I think we can reuse fairly amount of existing code (I don't look at code closely yet, but PrintContext::spoolPage and PixelDumpSupport seem to do most tricks). One of good points of PDF would be that boundaries of pages are very clear. I think we can easily add some lines like <hr> into the PNG image as the signs of page boundaries, so this won't be a serious lowlight of PNG approach. If this plan makes sense, I'd like to start implementing it. Any suggestions will be welcomed! printToPNG sounds like a good idea to me. We could even do a printToMPNG so we could store all the frames in one file. :) The problem with ImageDiff is that it's very port-specific. Is there a chance we could write a platform/port-independent ImageDiff that does all these things? Eric, didn't you say at some point that you wanted to use some sort of ruby library to do cross-platform image diffing. There's probably an equivalent in Python that we could use, too. I may have said such. I don't remember. There is a "cross-platform" image library ImageMagick which has ruby bindings. I expect it has python bindings. I quote "cross-platform" because when we used ImageMagick at my startup we had lots of trouble between platforms, based on what options you had compiled it with, or its variously 10s of dependent libraries. :) Ironically, simply decoding the bytes from a PNG should be incredibly portable. Maybe once we have gfx upstreamed we should just use that everywhere? -- Dirk Note that if we decide we can use the Python Imaging Library, producing a diff takes about 3 lines of code (although it may be slow): http://www.tilander.org/aurora/2008/03/comparing-images.html see http://athenageek.wordpress.com/2009/06/09/easy_install-pil-not-so-easy/ for the actual url to install PIL w/ easy_install. of course, that may not produce the kind of diff we'd like to see. Created attachment 55528 [details]
Patch v1
Attachment 55528 [details] did not build on win: Build output: http://webkit-commit-queue.appspot.com/results/2189118 (In reply to comment #10) > Attachment 55528 [details] did not build on win: > Build output: http://webkit-commit-queue.appspot.com/results/2189118 Hmm, I couldn't understand why this is failing from this build log. Maybe a bot issue? > Hmm, I couldn't understand why this is failing from this build log. Maybe a bot issue?
the win bot doesn't produce logs, sadly.
This patch is really cool. Who should review it? (In reply to comment #13) > This patch is really cool. Who should review it? Maybe Eric or Darin because they reviewed patch in Bug 37203? Hmm ping? Comment on attachment 55528 [details]
Patch v1
Seems OK.
WebCore/page/PrintContext.cpp:244
+ graphicsContext.drawLine(IntPoint(0, currentHeight),
+ IntPoint(pageWidth, currentHeight));
Not our usual formatting. Put it all on one line, or indent by 4 spaces. Please don't indent to match the parenthesis on the line above.
Committed r63521: <http://trac.webkit.org/changeset/63521> Committed r63522: <http://trac.webkit.org/changeset/63522> Committed r63523: <http://trac.webkit.org/changeset/63523> Committed r63525: <http://trac.webkit.org/changeset/63525> http://trac.webkit.org/changeset/63521 might have broken Tiger Intel Release The following changes are on the blame list: http://trac.webkit.org/changeset/63521 http://trac.webkit.org/changeset/63522 http://trac.webkit.org/changeset/63523 http://trac.webkit.org/changeset/63524 http://trac.webkit.org/changeset/63525 http://trac.webkit.org/changeset/63522 might have broken Tiger Intel Release The following changes are on the blame list: http://trac.webkit.org/changeset/63521 http://trac.webkit.org/changeset/63522 http://trac.webkit.org/changeset/63523 http://trac.webkit.org/changeset/63524 http://trac.webkit.org/changeset/63525 http://trac.webkit.org/changeset/63523 might have broken Tiger Intel Release The following changes are on the blame list: http://trac.webkit.org/changeset/63521 http://trac.webkit.org/changeset/63522 http://trac.webkit.org/changeset/63523 http://trac.webkit.org/changeset/63524 http://trac.webkit.org/changeset/63525 http://trac.webkit.org/changeset/63525 might have broken Tiger Intel Release The following changes are on the blame list: http://trac.webkit.org/changeset/63521 http://trac.webkit.org/changeset/63522 http://trac.webkit.org/changeset/63523 http://trac.webkit.org/changeset/63524 http://trac.webkit.org/changeset/63525 Committed r63526: <http://trac.webkit.org/changeset/63526> http://trac.webkit.org/changeset/63526 might have broken SnowLeopard Intel Release (Tests) Committed r63527: <http://trac.webkit.org/changeset/63527> Committed r63529: <http://trac.webkit.org/changeset/63529> |