Rework image snapping logic

RESOLVED FIXED

Status

()

RESOLVED FIXED
11 years ago
7 months ago

People

(Reporter: roc, Assigned: roc)

Tracking

Trunk
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments, 6 obsolete attachments)

What we have right now is complex and fragile. It kind of evolved but it doesn't really make sense.

I took a step back to figure out from bug reports and first principles what our real, precise requirements are and come up with an algorithm that satisfied those requirements. In fact, once I got the right requirements and specified them precisely, the algorithm is obvious.

https://wiki.mozilla.org/Gecko:Image_Snapping_and_Rendering

I'd appreciate feedback on the requirements and the algorithm.

Unlike what I said in bug 421438, I think it is actually possible to unify tiled and single-image drawing to produce identical rendering ... provided you choose the right "anchor point" for both cases.

I'd like to implement this now-ish. It will mean rewriting most of nsLayoutUtils::DrawImage, nsCSSRendering::PaintBackgroundSC, and nsThebesImage::DrawTile/Draw. Basically I think "the algorithm" from the above wiki page should be nsLayoutUtils::DrawImage. nsCSSRendering::PaintBackgroundSC will just compute the required parameters and pass them in. nsThebesImage::Draw will handle both tiled and non-tiled cases and take care of evil stuff like adjusting for image paddingX/Y and restricting the sampled pixels using EXTEND_PAD and temporary images or whatever.

The advantage of doing this is that it should fix some bugs and simplify our code, maybe even improve performance (since right now we could easily be scaling images by very small amounts when we shouldn't be). And in the future if we have snapping-related bugs we can refer to the wiki and figure out if the bug is a new requirement (in which case we have to decide how to fit it in among the other requirements and fix the algorithm, or reject it), or a bug in the reasoning of the above wiki page, or a bug in the code that we just have to fix.
Here's a problem, or at least an ambiguity, with the current requirements and algorithm: when scaling a tiled image, we might sample unintended pixels. E.g. suppose we have a 5px wide image supposed to cover a 15px wide area and scaled by 0.5x. The current formulation says it's OK to sample all the pixels of the image. We'll snap the 7.5px target area to 8px, and the last pixel of the rendered target will be the average of the rightmost pixel of the third image tile and the leftmost pixel of a fourth image tile. This is wrong because there is no fourth image tile in the ideal rendering.

So requirement 4 needs to be clarified/tightened to make it clear that it excludes situations like this, and the first step of the algorithm needs to be clarified too.
This requirement is actually a huge pain to implement in cairo and probably a source of bugs on trunk. Of course cairo can't do "repeat 3 times vertically and then pad".

An obvious way to fix it would be to compute the complete source image by tiling the image into a temporary surface, and then render that to the target using EXTEND_PAD, but that would be sucky for performance.

A better way to fix it would be to divide the area to fill into device-pixel-aligned regions that should be painted with a single tile and EXTEND_PAD, and an extra region which can be just filled.
Now I'm wrestling with the workaround for bug 364968. Downscaling the image is impossible to reconcile with the requirement that we don't sample outside the subimage rectangle in tile space.

Seems to me another way to fix bug 364968 would be to just do a PushGroup (after clipping to the device-pixel-aligned, rounded-out bounding box of the fill area). Then the offset of the fill rect from the surface will be small so the device-space-to-image-space transform matrix should be representable in 16.16. This would simplify the code and avoid messing with my invariants.
Posted patch fix v0 (obsolete) — Splinter Review
This patch builds and the browser basically works. I haven't done any systematic testing yet.

So far things worked out pretty much as I expected, except that the amount of code removed from nsCSSRendering was even more than I'd hoped for.
Posted patch patch v0.1 (obsolete) — Splinter Review
Known issues:

-- Fails to display reftest image modules/libpr0n/test/reftest/apng/bug411852-1.png correctly ... this is a weird one, modules/libpr0n/test/reftest/apng/bug411852-1-ref.png is fine

-- Fails reftest layout/reftests/bugs/433640-1.html ... we may just need to fix the test

(All other reftests pass!)

-- In test https://bugzilla.mozilla.org/attachment.cgi?id=320863, at some window widths the image leaks outside the border by one pixel

-- Red lines appear in https://bugzilla.mozilla.org/attachment.cgi?id=311143 at some zoom levels, so this hasn't fixed bug 446100 yet.
Attachment #342177 - Attachment is obsolete: true
(In reply to comment #5)
> -- Fails reftest layout/reftests/bugs/433640-1.html ... we may just need to fix
> the test

Agreed.  See bug 446100 comment 21.  Are the changes you need the same ones I needed in that patch?
Yes! Brilliant, thanks
How do you feel about adding built-in support for full zoom to the reftest harness? I think I can do the work.
(In reply to comment #5)
> -- Red lines appear in https://bugzilla.mozilla.org/attachment.cgi?id=311143 at
> some zoom levels, so this hasn't fixed bug 446100 yet.

Once you figure this out, I'm hoping the fix could also apply to a branch-safe fix for bug 446100 in addition to the patch I have there.

(In reply to comment #8)
> How do you feel about adding built-in support for full zoom to the reftest
> harness? I think I can do the work.

Sounds reasonable to me.  Was what you wanted something in the manifest like "zoom(1.75)"?
That's pretty much exactly what I had in mind.
Well, actually a better approach would let you specify separate zoom levels in the testcase and the reference. It seems likely we'll want to test both "zoomed testcase == unzoomed reference" and "zoomed testcase == zoomed reference".
So I think a reftest-zoom="1.75" attribute on the root element might be better. We've already got reftest-print and reftest-wait.
Note, this patch fixes background-origin on the root element, which is currently completely broken because the special-case code for computing the anchor point for the canvasFrame ignores bgOriginArea, bypassing the code that handles background-origin.
Posted patch fix v0.2 (obsolete) — Splinter Review
Adds zoom support to reftest.js, fixes a couple of bugs in the image patch, adds some tests.

Known issues:

-- *still* fails to display reftest image
modules/libpr0n/test/reftest/apng/bug411852-1.png correctly ...

-- Bug 446100 is fixed in this patch, but need to write a reftest based on https://bugzilla.mozilla.org/attachment.cgi?id=311143

-- Break out the reftest zoom support into a separate patch

Bug 446100 wasn't fixed in a simple extractable way; some logic in the new nsLayoutUtils::DrawImage (for setting the subimage size) was just very wrong.
Assignee: nobody → roc
Attachment #342196 - Attachment is obsolete: true
Filed bug 459619 with a patch for the reftest-zoom functionality.
Depends on: 459619
Posted patch fix v1 (obsolete) — Splinter Review
OK. This has reftests that pass (tested Linux and Mac), including tests for bug 446100. I found the problem with the APNG test, it was a simple error in the aPadding code path.

The patch is large, but it removes a lot more code than it adds. Here's a rundown:

-- nsIImage::DrawImage's signature changes to support tiling and arbitrary image transforms, and also takes the padding as a parameter as it will be entirely responsible for handling padding.

-- nsIRenderingContext::DrawTile and nsThebesImage::ThebesDrawTile are completely gone, since a single draw method handles everything now

-- nsThebesImage::Draw first handles the single-pixel, fully-decoded, not-padded case. (I haven't bothered writing special code for the single-pixel, padded-or-partially-decoded case, but it wouldn't be hard (for the non-tiled subcase, anyway) if someone thinks that's important.) Then we handle the partially-decoded-or-padded case; for a single tile we can just restrict the area we draw, but for tiling (and for the single-pixel case) we have to create a temporary surface containing the padding. Next we work around the pixman 16.16 bug by doing a PushGroup if necessary, which minimizes the offset of the image from the destination surface. Then, if resampling is going to happen, we handle the subimage area; we set EXTEND_PAD or something equivalent, and if the subimage is not just the image itself, we create a temporary image of the pixels that we're allowed to sample from. Then we draw the actual image, with the OPERATOR_SOURCE optimization if applicable. Note that nsThebesImage::Draw no longer does any pixel-snapping of its own.

-- Added HasNonIntegerTranslation to gfxMatrix

-- All the code in nsCSSRendering that tried to figure out how many tiles are necessary and where to start tiling is gone, it's no longer necessary.

-- Changed signature for nsCSSRendering::ComputeBackgroundAnchorPoint. We now return two points --- the top-left of the image, and the anchor point that needs to be pixel-aligned.

-- All pixel-snapping in nsCSSRendering::PaintBackgroundWithSC is gone, since nsLayoutUtils::DrawImage is now responsible for all pixel-snapping.

-- Renamed bgOriginArea to bgOriginRect and made it relative to aBorderArea.TopLeft()

-- Set bgOriginRect to the root element when we're painting the canvas background. This means we don't need a special case for canvas background painting later, and background-origin works correctly for the canvas background.

-- We no longer need to decide whether to tile or not, since it's the same path. Implementing background-repeat is now simply a matter of changing the fill rect appropriately.

-- nsLayoutUtils::DrawImage is now the algorithm described in the wiki page.

-- Created helpers nsLayoutUtils::DrawSingleUnscaledImage and nsLayoutUtils::DrawSingleImage

-- Created nsLayoutUtils::GetWholeImageDestination, which is useful for populating the aDest parameter of DrawImage when you just want to fill a rectangle with a subimage.

-- Various callers of nsLayoutUtils::DrawImage need to be fixed, mostly to use the helpers. The anchor point used is the image top-left --- there doesn't seem to be anything we can do to be smarter about that.

-- nsBaseDragService::DrawDragForImage needs to change to the new nsIImage::Draw API. I found srcRect and destRect were confusing, since they're always at 0,0, so I renamed them and changed them to nsSize.

I'd like David to review the layout bits and vlad to review the gfx bits.
Attachment #342707 - Attachment is obsolete: true
Attachment #342832 - Flags: superreview?(dbaron)
Attachment #342832 - Flags: review?(vladimir)
Attachment #342832 - Flags: review?(joe)
Comment on attachment 342832 [details] [diff] [review]
fix v1

I'd like to look at the gfx bits of this too -- adding to my review queue. I'll probably have Jeff Muizelaar look at it too.
Reftests pass on Windows too.

Comment 19

11 years ago
I tried this patch, but it doesn't work with HTML FRAMEs.
The patch looks to drop some code for the bug 46446, doesn't it?

Meanwhile, it works fine on pages without HTML FRAMEs. Great work!
I thought I handled that. What's your testcase?

Comment 21

11 years ago
Screenshot of the following testcase:

data:text/html,<html><frameset cols="20,*"><frame src="about:blank"><frame src="http://www.mozilla.org/"></frameset></html>

Comment 22

11 years ago
Screenshot of the following testcase:

data:text/html,<html><frameset rows="20,*"><frame src="about:blank"><frame
src="http://www.mozilla.org/"></frameset></html>

Comment 23

11 years ago
Sorry, I've forgotten my build ID (and my environment):
Mozilla/5.0 (Windows; U; Windows NT 6.0; ja; rv:1.9.1b2pre) Gecko/20081018 Minefield/3.1b2pre (.NET CLR 3.5.30729)

Now I tweaked the patch a bit so that it builds with latest trunk. (But I confirmed these problems I reported here on my older build with your original patch.)

I also noticed that specifying the first frames width or height more larger, Minefield crashes!
For example, in my environment the following URL crashed.
( >94 crashes too.)

data:text/html,<html><frameset rows="94,*"><frame src="about:blank"><frame src="http://www.mozilla.org/"></frameset></html>
I don't get that crash. I've got a patch for the main issue you reported, thanks for trying the patch!
Posted patch fix v2 (obsolete) — Splinter Review
That frame/iframe problem was because I wasn't adjusting the dirty rect to account for translation in the snapped-rectangle case in nsLayoutUtils::DrawImage.

Also, I realized that when we're not snapping the rectangle, especially for the general transform case, we can't really use the dirty rect at all, because if the dirty rect is not snapped perfectly to device pixel boundaries, some device pixels in the fill area will be partially inside and partially outside the "dirty rect", so the values painted for those pixels will depend on the dirty rect, which is a disaster.
Attachment #342832 - Attachment is obsolete: true
Attachment #343861 - Flags: superreview?(dbaron)
Attachment #343861 - Flags: review?(vladimir)
Attachment #342832 - Flags: superreview?(dbaron)
Attachment #342832 - Flags: review?(vladimir)
Attachment #342832 - Flags: review?(joe)

Comment 26

11 years ago
I'm testing the patch v2, the problem I've reported has been fixed. (including the crash problem, I think it might be a crash in the gfx layer by drawing images on inappropriate position.)
Thanks a lot! 

And now it also fixes bug 446100 perfectly!
Comment on attachment 343861 [details] [diff] [review]
fix v2

Image/gfx changes look fine -- much easier to follow now than the previous workaround upon workaround.  You may want to add a gfxMatrix Inverse() const helper to solve your Invert() problem.

Also, why the bool->PRBool change? Wrong direction! :)
Attachment #343861 - Flags: review?(vladimir) → review+
Arguably yes, but we need to switch to bool via some big-bang Taras-powered change; piecemeal slippage just confuses things IMHO. But you're the module owner here so I'll leave those alone if you insist.
(In reply to comment #28)
> Arguably yes, but we need to switch to bool via some big-bang Taras-powered
> change; piecemeal slippage just confuses things IMHO. But you're the module
> owner here so I'll leave those alone if you insist.

Nah, it's fine to switch it -- there's already an unfortunate mix of bool/PRBool there, so I don't have any strong preference either way until we fix it outright.
Attachment #343861 - Flags: review?(joe) → review+
Comment on attachment 343861 [details] [diff] [review]
fix v2

>+            // Create a temporary surface
>+            gfxIntSize size(PRInt32(imageRect.Width()),
>+                            PRInt32(imageRect.Height()));
>+            // Give this surface an alpha channel because there are
>+            // transparent pixels in the padding or undecoded area
>+            format = gfxASurface::ImageFormatARGB32;
>+            surface = gfxPlatform::GetPlatform()->CreateOffscreenSurface(size,
>+                format);
>+            if (!surface || surface->CairoStatus() != 0)
>+                return;

I am slightly concerned about this in the case of huge images. CreateOffscreenSurface should probably handle it properly, though, so it's not a huge concern.

>+    // BEGIN working around cairo/pixman bug (bug 364968)
[......]
>+    // END working around cairo/pixman bug (bug 364968)

Is it possible to put this into a helper function to hide its ugliness?
(In reply to comment #30)
> (From update of attachment 343861 [details] [diff] [review])
> >+            // Create a temporary surface
> >+            gfxIntSize size(PRInt32(imageRect.Width()),
> >+                            PRInt32(imageRect.Height()));
> >+            // Give this surface an alpha channel because there are
> >+            // transparent pixels in the padding or undecoded area
> >+            format = gfxASurface::ImageFormatARGB32;
> >+            surface = gfxPlatform::GetPlatform()->CreateOffscreenSurface(size,
> >+                format);
> >+            if (!surface || surface->CairoStatus() != 0)
> >+                return;
> 
> I am slightly concerned about this in the case of huge images.
> CreateOffscreenSurface should probably handle it properly, though, so it's not
> a huge concern.

Hopefully it will just fail. It's bad that we'll create an image that's as big as the element, which could be arbitrarily large, but there is really no other way to do this without source clipping in cairo. Because if we try to take the damage rect into account to determine the temporary image size, we're likely to get results that depend on the damage rect.

> >+    // BEGIN working around cairo/pixman bug (bug 364968)
> [......]
> >+    // END working around cairo/pixman bug (bug 364968)
> 
> Is it possible to put this into a helper function to hide its ugliness?

I'll have a go.
There are some reftests marked random-if(MOZ_WIDGET_TOOLKIT="cocoa") in layout/reftests/pixel-rounding/reftest.list that should be fixed by the sampling region restriction stuff implemented in this bug; it would probably be good if you removed those random-if() notations.
After I've landed this, I'll retest those tests.
Comment on attachment 343861 [details] [diff] [review]
fix v2

>     case NS_STYLE_BG_INLINE_POLICY_EACH_BOX:
>-      bgOriginArea = aBorderArea;
>+      bgOriginRect.SizeTo(aBorderArea.Size());
>       break;

It might be clearer to do bgOriginRect.SetRect(nsPoint(0, 0), aBorderArea.Size()).

>+  } else {
>+    bgOriginRect.SizeTo(aBorderArea.Size());
>   }

Same here.

>   // Background images are tiled over the 'background-clip' area
>   // but the origin of the tiling is based on the 'background-origin' area
>   if (aColor.mBackgroundOrigin != NS_STYLE_BG_ORIGIN_BORDER) {
>-    nsMargin border = aForFrame->GetUsedBorder();
>-    aForFrame->ApplySkipSides(border);
>-    bgOriginArea.Deflate(border);
>+    nsMargin border = geometryFrame->GetUsedBorder();
>+    geometryFrame->ApplySkipSides(border);
>+    bgOriginRect.Deflate(border);
>     if (aColor.mBackgroundOrigin != NS_STYLE_BG_ORIGIN_PADDING) {
>-      nsMargin padding = aForFrame->GetUsedPadding();
>-      aForFrame->ApplySkipSides(padding);
>-      bgOriginArea.Deflate(padding);
>+      nsMargin padding = geometryFrame->GetUsedPadding();
>+      geometryFrame->ApplySkipSides(padding);
>+      bgOriginRect.Deflate(padding);
>       NS_ASSERTION(aColor.mBackgroundOrigin == NS_STYLE_BG_ORIGIN_CONTENT,
>                    "unknown background-origin value");
>     }
>   }

Do we have tests for background-origin on the root element and on the body element?  When that background is applied to the canvas, both should depend on the root element's geometry.  If not, could you add some?

(I'm a little worried because aBorderArea doesn't match the border area of geometryFrame.)

It's also worth testing background positioning with background-position:fixed (positioning relative to the viewport, and ignoring background-origin) -- both for backgrounds propagated to the canvas and those not (and various values of background-origin).  Though your code doesn't touch that much, so it's probably ok to defer those if you prefer.

>+  nsRect destArea = nsRect(imageTopLeft, imageSize) + aBorderArea.TopLeft();

Seems better to write this as a constructor rather than an assignment.  (You could add the points inside the first argument.)
Comment on attachment 343861 [details] [diff] [review]
fix v2

>+static gfxPoint
>+MapToFloatImagePixels(const nsIntSize& aSize,
>+                      const nsRect& aDest, const nsPoint& aPt)
>+{
>+  return gfxPoint(gfxFloat(aPt.x - aDest.x)*aSize.width/aDest.width,
>+                  gfxFloat(aPt.y - aDest.y)*aSize.height/aDest.height);
>+}

Could you toss in some parentheses so it's clear what order of operations you intend?  (I'm presuming it's the one C++ gives you, where you do everything as floating point, but adding parentheses helps to show that it matters here due to the different types involved.)
Comment on attachment 343861 [details] [diff] [review]
fix v2

Is the new code in nsTreeBodyFrame::PaintImage compatible with the old code?  Might it be worth keeping some of the comment?
(In reply to comment #34)
> (From update of attachment 343861 [details] [diff] [review])
> >     case NS_STYLE_BG_INLINE_POLICY_EACH_BOX:
> >-      bgOriginArea = aBorderArea;
> >+      bgOriginRect.SizeTo(aBorderArea.Size());
> >       break;
> 
> It might be clearer to do bgOriginRect.SetRect(nsPoint(0, 0),
> aBorderArea.Size()).

OK

> >+  } else {
> >+    bgOriginRect.SizeTo(aBorderArea.Size());
> >   }
> 
> Same here.

OK

> >   // Background images are tiled over the 'background-clip' area
> >   // but the origin of the tiling is based on the 'background-origin' area
> >   if (aColor.mBackgroundOrigin != NS_STYLE_BG_ORIGIN_BORDER) {
> >-    nsMargin border = aForFrame->GetUsedBorder();
> >-    aForFrame->ApplySkipSides(border);
> >-    bgOriginArea.Deflate(border);
> >+    nsMargin border = geometryFrame->GetUsedBorder();
> >+    geometryFrame->ApplySkipSides(border);
> >+    bgOriginRect.Deflate(border);
> >     if (aColor.mBackgroundOrigin != NS_STYLE_BG_ORIGIN_PADDING) {
> >-      nsMargin padding = aForFrame->GetUsedPadding();
> >-      aForFrame->ApplySkipSides(padding);
> >-      bgOriginArea.Deflate(padding);
> >+      nsMargin padding = geometryFrame->GetUsedPadding();
> >+      geometryFrame->ApplySkipSides(padding);
> >+      bgOriginRect.Deflate(padding);
> >       NS_ASSERTION(aColor.mBackgroundOrigin == NS_STYLE_BG_ORIGIN_CONTENT,
> >                    "unknown background-origin value");
> >     }
> >   }
> 
> Do we have tests for background-origin on the root element and on the body
> element?  When that background is applied to the canvas, both should depend on
> the root element's geometry.  If not, could you add some?

458487-1{a,b,c,d}.html test background-origin and background-position on the root element. I'll add an additional set of tests that test them on the body element.

> It's also worth testing background positioning with background-position:fixed
> (positioning relative to the viewport, and ignoring background-origin) -- both
> for backgrounds propagated to the canvas and those not (and various values of
> background-origin).  Though your code doesn't touch that much, so it's probably
> ok to defer those if you prefer.

I'll try adding some tests, but I'll file a followup bug if they don't work.

> >+  nsRect destArea = nsRect(imageTopLeft, imageSize) + aBorderArea.TopLeft();
> 
> Seems better to write this as a constructor rather than an assignment.  (You
> could add the points inside the first argument.)

Yes.

(In reply to comment #35)
> (From update of attachment 343861 [details] [diff] [review])
> >+static gfxPoint
> >+MapToFloatImagePixels(const nsIntSize& aSize,
> >+                      const nsRect& aDest, const nsPoint& aPt)
> >+{
> >+  return gfxPoint(gfxFloat(aPt.x - aDest.x)*aSize.width/aDest.width,
> >+                  gfxFloat(aPt.y - aDest.y)*aSize.height/aDest.height);
> >+}
> 
> Could you toss in some parentheses so it's clear what order of operations you
> intend?  (I'm presuming it's the one C++ gives you, where you do everything as
> floating point, but adding parentheses helps to show that it matters here due
> to the different types involved.)

Sure.

(In reply to comment #36)
> (From update of attachment 343861 [details] [diff] [review])
> Is the new code in nsTreeBodyFrame::PaintImage compatible with the old code? 
> Might it be worth keeping some of the comment?

Yes, and yes, good point. The first part of the comment is obsolete but the second part, the description of the effects, is still valid.
Comment on attachment 343861 [details] [diff] [review]
fix v2

I might try to look at this a little more later, but sr=dbaron with that stuff fixed.
Attachment #343861 - Flags: superreview?(dbaron) → superreview+
Comment on attachment 343861 [details] [diff] [review]
fix v2

And might it be worth using UserToDevicePixelSnapped to snap the anchor point rather than using UserToDevice and then Round?  Doesn't that fix the comment about the assumption you're making?

(sorry, had this in an open edit-attachment window and forgot to submit earlier)
Yes, that's a good idea.
(In reply to comment #31)
> (In reply to comment #30)
> > >+    // BEGIN working around cairo/pixman bug (bug 364968)
> > [......]
> > >+    // END working around cairo/pixman bug (bug 364968)
> > 
> > Is it possible to put this into a helper function to hide its ugliness?
> 
> I'll have a go.

This turns out to be difficult because the helper really has three possible results: "pixman can't handle this, abort out", "pushed a group, continue and then pop the group", and "continue". So I'm punting it.
Posted patch fix v3 (obsolete) — Splinter Review
With comments addressed. I was going to check this in but the tree hasn't been green enough.
Posted patch fix v4Splinter Review
Without the stray nsTableFrame change.
Attachment #343861 - Attachment is obsolete: true
Attachment #346236 - Attachment is obsolete: true
Attachment #346332 - Flags: superreview+
Attachment #346332 - Flags: review+
Changeset 49dd935efff3
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Flags: in-testsuite+
Resolution: --- → FIXED

Comment 45

11 years ago
Posted image clipped emoticons
In my SM2 i noticed, that the left side of emticons is clipped and the missing part is attached to the right side. Backing out fix v4 solves the problem.

Updated

11 years ago
Blocks: 463204

Comment 46

11 years ago
(In reply to comment #45)
> Created an attachment (id=346435) [details]
> clipped emoticons
> 
> In my SM2 i noticed, that the left side of emticons is clipped and the missing
> part is attached to the right side. Backing out fix v4 solves the problem.

Have filed Bug 463204 for this issue, cc'ing roc as he knows best about his patch.
No longer blocks: 463204
Depends on: 463204

Comment 47

11 years ago
This fix is no good. In 456330, the background image is completely gone and doesn't render.
I found the problem in bug 456330 and put up a patch. I need a testcase for bug 463204, please attach one there.

Updated

11 years ago
Depends on: 463307
Blocks: 424672

Comment 49

11 years ago
This appears to have also caused bug 463217

Comment 50

11 years ago
Yes, I think.
My attachment of this bug (attachment (id=343683)) has already shown that bug (bug 463217) but I couldn't catch that...

Updated

11 years ago
Blocks: 410180
Depends on: 463666
Depends on: 463938
Depends on: 466258
No longer depends on: 466258
Depends on: 466258

Updated

11 years ago
Blocks: 467029

Comment 51

11 years ago
This also probably caused bug 467029.
So can you have a look on that bug, as that is a serious regression in background image drawing. And there is no known workaround for it.
No longer blocks: 467029
Depends on: 467029
Depends on: 478560

Comment 52

10 years ago
Potential regression on Linux: bug 487996

Updated

10 years ago
Blocks: 230101
Depends on: 729729

Updated

7 months ago
Product: Core → Core Graveyard

Updated

7 months ago
Product: Core Graveyard → Core
You need to log in before you can comment on or make changes to this bug.