We're incorrectly truncating in a few places.
Created attachment 159780 [details] Patch Patch.
Probably best for you to explain this to me in person.
(In reply to comment #2) > Probably best for you to explain this to me in person. Tomorrow?
Ping.
Sorry, I missed you before burning man. Tomorrow, for realz this time.
Comment on attachment 159780 [details] Patch Levi and I talked about this at length this afternoon. This fix is essentially trying to follow the intent of the author in the case of zooming, even when we don't have all the necessary information. The problem starts because background-images by default tile instead of scale, yet authors often specify a background image which is exactly the size of their object. If we happen to be scaling the page, and size of that object could deviate from the size of the background image by as much as one pixel. In that case, we'll tile the background image, which be believe is likely not what the author intended. The fix currently proposed is an attempt to make our background image tiling code match approximately what the rendering tree does with the divs. This is of course impossible, since he tiles are always drawn of uniform size, and the objects in the rendering tree may gain pixels based on accumulated pixel offsets from the zoom. I suspect that even with this proposed patch we could still have this problem, as we're using round() here instead of ciel(). ciel() would make it so that we were never too small, but would more often be too big. The previous code truncated which would make the background image most often too small, and never too big. round() gives us a little bit of both still. :) Another approach to this might be to add special code for the painting of background images, which attempted to detect when we're drawing a background image which is one pixel smaller than the div, and specified to tile, and instead change it to scale in that case. That heuristic would solve this observed problem during zooming, and might be closer to what the authors intend. It's a heuristic, and would obviously deviate from the CSS spec, but might be the "right" thing to do.
Comment on attachment 159780 [details] Patch r-, per above. I also think we could get better testing of this exact case pretty easily which showed the difference between old (floor) behavior, round() and ciel()? I'm happy to help you come up with such if you like.
Created attachment 164611 [details] Patch
Comment on attachment 164611 [details] Patch I think this looks great. My only comments would be: 1. We should make the function name (or at least commetns in the code) make clear that this is a heuristic we're applying (aka, not following any spec, or what you might expect the math to do). 2. I would limit this to the common case where the tile repeats exactly once, and we're one pixel off. (Do we need to support either way? both floor/ and ciel?) I guess the image is never a subpixel width?
Created attachment 171087 [details] Patch
Comment on attachment 171087 [details] Patch The updated patch addresses the concerns mentioned earlier and the new test is more to the point. Looks good to me.
Comment on attachment 171087 [details] Patch Rejecting attachment 171087 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: content): Merge conflict in LayoutTests/ChangeLog Failed to merge in the changes. Patch failed at 0001 Pixel tests need rebaseline https://bugs.webkit.org/show_bug.cgi?id=99323 When you have resolved this problem run "git rebase --continue". If you would prefer to skip this patch, instead run "git rebase --skip". To restore the original branch and stop rebasing run "git rebase --abort". rebase refs/remotes/origin/master: command returned error: 1 Died at Tools/Scripts/update-webkit line 154. Full output: http://queues.webkit.org/results/14610477
Comment on attachment 171087 [details] Patch Grumble grumble ChangeLogs grumble grumble.
Comment on attachment 171087 [details] Patch Clearing flags on attachment: 171087 Committed r132731: <http://trac.webkit.org/changeset/132731>
All reviewed patches have been landed. Closing bug.