Summary: | r93580 changed Win/Linux test results, changes not obviously correct | ||
---|---|---|---|
Product: | WebKit | Reporter: | Peter Kasting <pkasting> |
Component: | Images | Assignee: | John Bates <jbates> |
Status: | RESOLVED FIXED | ||
Severity: | Normal | CC: | brettw |
Priority: | P2 | ||
Version: | 528+ (Nightly build) | ||
Hardware: | Unspecified | ||
OS: | Unspecified | ||
Bug Depends on: | |||
Bug Blocks: | 65587 |
Description
Peter Kasting
2011-08-23 12:30:41 PDT
Thanks Peter, yes I think these tests should be rebaselined. There have always been two resize paths for drawResampledBitmap: 1. resize whole image and paint into whole destRect (let skia clip it). 2. resize only visible subset of cropped image and paint it into clipped destRect. There was a bug in the original code which caused the decision of path (1) or (2) to be harmlessly incorrect. Also, path (1) was only ever taken for uncropped images (srcImage == whole image). These test results are different because patch 65587 changes the decision making between path (1) and (2). The behavior is also slightly different for cropped images: 1. resize cropped image and paint visible subset into clipped destRect. 2. resize visible subset of cropped image and paint it into clipped destRect. The new code always paints the visible subset into the clipped-to-canvas destRect. In the original code, path 1 would paint the whole resized image into the whole destRect (allowing skia code to do the clip) and path 2 would paint the visible subset into the clipped-to-canvas destRect. Why aren't these painting techniques identical? I think it's because the platform/skia ClipToCanvas code is inaccurate -- it converts between int and float to get around a limitation of an int-only skia clip function. I think this would explain the 1-off crap. Ideally, we stay in float space for as long as possible, but we're going to have to add some additional float support in skia and then fix the platform/skia code. (In reply to comment #1) > Thanks Peter, yes I think these tests should be rebaselined. OK, I'll take care of it. > I think it's because the platform/skia ClipToCanvas code is inaccurate -- it converts between int and float to get around a limitation of an int-only skia clip function. I think this would explain the 1-off crap. Ideally, we stay in float space for as long as possible, but we're going to have to add some additional float support in skia and then fix the platform/skia code. Is there a bug on this somewhere? Should this be that bug? I want to make sure that if something needs to be fixed eventually here, we don't lose that data. (In reply to comment #2) > Is there a bug on this somewhere? Should this be that bug? I want to make sure that if something needs to be fixed eventually here, we don't lose that data. Absolutely, just filed: http://code.google.com/p/skia/issues/detail?id=345 https://bugs.webkit.org/show_bug.cgi?id=66810 |