I cannot reproduce, stable 55.0.2883.91 nor canary. An image of a person riding a bike is displayed for me at http://163.172.176.85/testcase.html, confirmed on Pixel C (Android 7.1.1) and Win10.

Are there any other possible factors to reproduce?

Closing as WontFix, as not being able to reproduce and no further feedback. Please add comments if any.

Hi Kojii, sorry I didn't see your original comment asking for feedback. I extracted the test case from here: https://www.wikiloc.com/trails/outdoor/united-states

If you open the page from Chrome mobile, the sprites aren't shown. Please see the screenshot: http://i.imgur.com/nKE8PJC.png

The same page works in Chrome for iOS, Safari mobile and Firefox mobile. But it doesn't work in Opera mobile.

Deleted: Screenshot_2017-01-24-10-09-01.png 243 KB

	Screenshot_2017-01-24-10-09-01.png 243 KB View Download

#5: Thank you for the feedback, confirmed at
https://www.wikiloc.com/trails/outdoor/united-states
using Canary 57.0.2983.0

We are able to repro this issue on Chrome Stable: 56.0.2924.87 Device:Samsung Galaxy S6 (SM-G920F) / MMB29K 

Good Build:53.0.2785.97
Bad Build: 54.0.2790.2

https://chromium.googlesource.com/chromium/src/+log/53.0.2785.0..54.0.2790.0?pretty=fuller&n=10000

Bisect script pointed to https://chromium.googlesource.com/chromium/src/+/a12be529fb88039cc3090a83c4fbaaf04e13b144

I think this is a case of a bad bisect. The commit it pointed to updates our linting logic and has no effect on what comprises the final Chrome.apk.

per-CL bisect is not possible since there are no archived per-cl builds available for this range because this are old builds 
Good Build:53.0.2785.97
Bad Build: 54.0.2790.2

https://chromium.googlesource.com/chromium/src/+log/53.0.2785.0..54.0.2790.0?pretty=fuller&n=10000

Removing the border and border-radius on this span makes the images start to appear:

<span class="pictogram pictogram-rounded activity-2" title="Mountain biking"></span>

The element is not composited, so changing to Blink>Paint.

I have a local repro (still only on Android sadly), but it's enough to be able to
reduce it.

Reduced testcase attached. a mobile meta viewport like:

<meta name="viewport" content="">

is required to trigger the bug. It also seems to have something
to do with float:left

Deleted: testcase.html 287 bytes

testcase.html 287 bytes View Download

Hello,

Wikiloc dev here. I will change the CSS of the website next Monday and remove the borders of the sprite so that users at least can see the activity icon. Please use the reduced test case from now on.

Thank you.

This is a bug in GPU raster - forcing it off shows the icon.
I think the issue might be that we are exceeding the maximum texture size,
kicking us into some other raster path which then has a bug.

Eric could you take this one?

hi, please see
https://bugs.chromium.org/p/chromium/issues/detail?id=780406

With Gpu rasterization, and draw image using SkCanvas::drawRect, this issue will occurs.
Root cause is that image size exceed max texture size, and GrGpu::createTexture reture nullptr.


Below is backtace:
#0  GrGpu::createTexture () at ../../third_party/skia/src/gpu/GrGpu.cpp:143
#1  0x98e6db52 in GrTextureProvider::createMipMappedTexture () at ../../third_party/skia/src/gpu/GrTextureProvider.cpp:77
#2  0x98e6dcb0 in GrTextureProvider::createMipMappedTexture () at ../../third_party/skia/src/gpu/GrTextureProvider.cpp:88
#3  GrTextureProvider::createTexture () at ../../third_party/skia/src/gpu/GrTextureProvider.cpp:91
#4  0x98ed64e8 in GrUploadPixmapToTexture () at ../../third_party/skia/src/gpu/SkGr.cpp:181
#5  0x98ed67a0 in GrUploadBitmapToTexture () at ../../third_party/skia/src/gpu/SkGr.cpp:121
#6  0x98e47894 in GrBitmapTextureMaker::refOriginalTexture () at ../../third_party/skia/src/gpu/GrBitmapTextureMaker.cpp:43
#7  0x98e6ce3c in GrTextureMaker::refTextureForParams () at ../../third_party/skia/src/gpu/GrTextureMaker.cpp:29
#8  0x98ed5ec4 in GrRefCachedBitmapTexture () at ../../third_party/skia/src/gpu/SkGr.cpp:284
#9  0x98dcdb4a in SkImage_Raster::asTextureRef () at ../../third_party/skia/src/image/SkImage_Raster.cpp:194
#10 0x98dce7ae in SkImageShader::asFragmentProcessor () at ../../third_party/skia/src/image/SkImageShader.cpp:176
#11 0x98ed7036 in skpaint_to_grpaint_impl () at ../../third_party/skia/src/gpu/SkGr.cpp:452
#12 0x98ed7360 in SkPaintToGrPaint () at ../../third_party/skia/src/gpu/SkGr.cpp:586
#13 0x98ed2c00 in SkGpuDevice::drawRect () at ../../third_party/skia/src/gpu/SkGpuDevice.cpp:412
#14 0x98d3118a in SkCanvas::onDrawRect () at ../../third_party/skia/src/core/SkCanvas.cpp:2164
#15 0x98e3c55e in SkNWayCanvas::onDrawRect () at ../../third_party/skia/src/utils/SkNWayCanvas.cpp:149
#16 0x98cab134 in cc::ImageHijackCanvas::onDrawRect (this=0x93b814ac, r=..., paint=...) at ../../cc/playback/image_hijack_canvas.cc:282
#17 0x98da1df8 in SkRecord::visit<SkRecords::Draw&> () at ../../third_party/skia/src/core/SkRecord.h:51
#18 SkRecordDraw () at ../../third_party/skia/src/core/SkRecordDraw.cpp:54
#19 0x98d1a92c in SkBigPicture::playback () at ../../third_party/skia/src/core/SkBigPicture.cpp:41
#20 0x98d351bc in SkCanvas::onDrawPicture () at ../../third_party/skia/src/core/SkCanvas.cpp:3086
#21 0x98d3527c in SkCanvas::drawPicture () at ../../third_party/skia/src/core/SkCanvas.cpp:3066
#22 0x98caa248 in SkCanvas::drawPicture (picture=<optimized out>, this=0x93b814ac) at ../../third_party/skia/include/core/SkCanvas.h:1050
#23 cc::DrawingDisplayItem::Raster (this=0x8ca5d00c, canvas=0x93b814ac, callback=0x0) at ../../cc/playback/drawing_display_item.cc:95

Issue 780406 has been merged into this issue.

Any update on this one?

Looking at this again in the context of the callstack from #20, it appears that this is a Skia issue - the Gpu Image Decode controller is handing this off to skia as a Bitmap image (likely because we've detected larger than max texture size and haven't tried to upload ourselves. For some reason Skia then tries to upload in a way that fails.

bsalomon@, can you help triage?

If draw the *big* image using SkGpuDevice::drawBitmapRect, then will process by SkGpuDevice::drawTiledBitmap, and will break the *big* image into several tiles to draw. This path is correct 

Friendly ping - hcm@, is there someone on the Skia side who can take a look per the comment in #24?

Sorry this fell off my radar. Currently when an image shader has an image that exceeds the max texture size we drop the draw entirely. We are planning to change that behavior to execute the draw but ignore the shader. Making image shaders work in general with images larger than the max texture size is a large project and is not planned.

What will be displayed if the shader is ignored for the testcase in comment
17? An unclipped sprite image?

I understand that in the general shader case this can be tricky, but I wonder if most of these user scenarios need to use a shader at all? In the case seen here, we're basically just clipping a large image. A few questions:

- Is there a reason Blink can't generate a DrawImageRect instead of a shader in simplace cases? (bsalomon@, is my understanding that DrawImageRect would work correct)?
- Alternately, could Skia detect simple scale/clip shaders and convert these to DrawImageRect internally? I understand that solving the general case is a large project, but I wonder if a simple workaround would solve most cases?

Re: #28, It would be a rectangle with the color assigned to the SkPaint (probably solid white).

Re #29, In this case transforming to an image draw would fix this. I'd prefer to avoid that kind of SkShader introspection in Skia if that conversion could be done up in blink. +fmalita@ regarding how easy that is.

Yeah, let's not undo the shader heuristically in Skia...

... 'cause we're working pretty hard to produce it heuristically in Blink :)

https://codereview.chromium.org/1949253004

I've added the shader path because imageShader + drawRRect was significantly faster than clipRRect + drawImage with Ganesh.  

Since it's supposed to be a fast path, it would be easy to refine the preconditions and fall back to clipping when the image is too large (assuming we have a good definition for "too large").

But (while I haven't looked too closely at this case) I think the problem here is we're creating a shader for the whole atlas.  Subsetting only the interesting part for the shader would presumably also avoid the issue.

Will investigate.

If it helps, adding a faster path to handle clipRRect + drawImage where the rrect is inside the image bounds would be pretty easy to add to Ganesh since it doesn't require paint introspection. We have something similar for clipRRect + drawPaint already.

That would be great, since the scaffolding to support this in Blink is quite involved.

As a stop gap, if we wanted to veto the shader path based on image size, what's a good way to determine max texture size?  Or a reasonable constant?

It's accessible from GrContext::caps()->maxTextureSize(). If that isn't handy the minimum maximum allowed by ES2 is 2048, though the actual limit is often quite a bit higher.

I can probably do the clipRRect + drawImage by end of day if that changes your decision making about how to go about this.

There's no GrContext available at that stage, we'd have to use 2048.

Having the drawImage optimization in Skia seems desirable, I think we should do that regardless.

But I just realized that the shader fast path is more general, not just for images - also applies to stuff like gradients (which is where the biggest gains are).  So we'd probably have to consider adding to drawRect() and possibly other primitives.

I think we should land the size-based veto right away to fix this issue, but continue pursuing Skia power reductions (for SW also?).  When everything is in place, we can drop the fast path from Blink and remove a bunch of scaffolding.

chrishtr@, ericrk@ WDYT?

Re landing the veto first: makes sense to me. I also like the plan to pursue
Skia power reductions and then reduce complexity in BoxPainter.

Thanks!

I have a CL to veto DrawImageRRect-as-shader for large images (https://chromium-review.googlesource.com/c/chromium/src/+/803934), but it doesn't fix the observed issue :(

Turns out the rounded rect fast path is just the first layer.

The second issue: for some reason, the computed image geometry for the test in c#17 seems incorrect (the image geometry appears too narrow to cover the dest rect, while in reality it isn't), causing  PaintFastBottomLayer to bail ([1]) and we take the background tiling fallback - which again uses image shaders.

[1] https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/paint/BoxPainterBase.cpp?rcl=33c044c77ffdbae8363b1a3426edb3099e38a855&l=396

But this got me thinking: even if we fix these issues, and make this particular test pass, there are legitimate cases where background tiling is unavoidable, and that involves on shaders.

Trivial example with the same atlas, failing on mobile devices: https://codepen.io/fmalita/pen/ZaVNvQ?editors=1100

We can probably infer a subset for the shader in this case, but the actual subsetting needs to be deferred to avoid premature decoding.

So not a trivial workaround after all.

Re comment 38: ok. What is a good next step then?

Looking at a couple of things:

1) detect and avoid unnecessary tiling

The presence of borders triggers background tiling by default (background-image-origin: padding-box; background-repeat: repeat;) because background images are tiled logically underneath borders also.  But if the borders are opaque, we can skip these invisible tiles and avoid image shaders in some cases.

This may be a decent fix for the original report.

2) subset image shaders

We could detect that only a subset of the original image is used in the shader, and extract before rasterization.

This is trickier to implement and has some unclear implications (*), but would solve the general tiling case of c#38.

* Is it always better to subset? Presumably we're uploading these subsets independently, so for general atlassing it seems sub-optimal.


It's probably also worth noting: as a workaround, authors should attempt to keep the atlas dimensions balanced (e.g. 440x440 instead of 40x4760 in this case), to avoid running into texture limitations on mobile devices.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d46daceb7047d0ab92a164a062fa26e31baae377

commit d46daceb7047d0ab92a164a062fa26e31baae377
Author: Florin Malita <fmalita@chromium.org>
Date: Wed Dec 06 18:00:13 2017

Avoid unnecessary background image tiling

Currently BackgroundImageGeometry computes a tiling geometry in the
presence of borders.  This is correct, because (at least for
background-origin: padding-box/content-box) backgrounds are repeated
logically underneath the border.

But if the border is opaque and fully obscures the background, these
extra tiles are redundant.  What's more, tiling forces us to use
image shaders for painting, unnecessarily.

This CL introduces logic to minimize the background draw rect, and avoid
unnecessary tiling for opaque borders.

Also, because it is mostly used as an offset, replace box_outset with
box_offset in BackgroundImageGeometry::Calculate.

BUG= 676636 

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I6b7095036a197df104fb34422d33943f5901aa27
Reviewed-on: https://chromium-review.googlesource.com/809229
Commit-Queue: Florin Malita <fmalita@chromium.org>
Reviewed-by: Stephen Chenney <schenney@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522133}
[modify] https://crrev.com/d46daceb7047d0ab92a164a062fa26e31baae377/third_party/WebKit/LayoutTests/fast/backgrounds/gradient-background-leakage-2-expected.png
[modify] https://crrev.com/d46daceb7047d0ab92a164a062fa26e31baae377/third_party/WebKit/LayoutTests/fast/backgrounds/gradient-background-leakage-expected.png
[modify] https://crrev.com/d46daceb7047d0ab92a164a062fa26e31baae377/third_party/WebKit/LayoutTests/fast/hidpi/gradient-with-scaled-ancestor-expected.png
[modify] https://crrev.com/d46daceb7047d0ab92a164a062fa26e31baae377/third_party/WebKit/LayoutTests/fast/table/border-radius-with-image-expected.png
[modify] https://crrev.com/d46daceb7047d0ab92a164a062fa26e31baae377/third_party/WebKit/LayoutTests/platform/linux/fast/gradients/conic-gradient-out-of-range-expected.png
[modify] https://crrev.com/d46daceb7047d0ab92a164a062fa26e31baae377/third_party/WebKit/LayoutTests/platform/linux/fast/gradients/crash-on-zero-radius-expected.png
[modify] https://crrev.com/d46daceb7047d0ab92a164a062fa26e31baae377/third_party/WebKit/LayoutTests/platform/linux/fast/gradients/simple-gradients-expected.png
[modify] https://crrev.com/d46daceb7047d0ab92a164a062fa26e31baae377/third_party/WebKit/LayoutTests/platform/linux/fast/gradients/unprefixed-repeating-gradient-color-hint-expected.png
[modify] https://crrev.com/d46daceb7047d0ab92a164a062fa26e31baae377/third_party/WebKit/LayoutTests/platform/mac-mac10.10/fast/forms/select/select-style-expected.png
[modify] https://crrev.com/d46daceb7047d0ab92a164a062fa26e31baae377/third_party/WebKit/LayoutTests/platform/mac-mac10.11/fast/forms/select/select-style-expected.png
[modify] https://crrev.com/d46daceb7047d0ab92a164a062fa26e31baae377/third_party/WebKit/LayoutTests/platform/mac/css3/background/background-large-position-and-size-remains-stable-expected.png
[modify] https://crrev.com/d46daceb7047d0ab92a164a062fa26e31baae377/third_party/WebKit/LayoutTests/platform/mac/css3/masking/mask-repeat-round-content-expected.png
[modify] https://crrev.com/d46daceb7047d0ab92a164a062fa26e31baae377/third_party/WebKit/LayoutTests/platform/mac/css3/masking/mask-repeat-space-padding-expected.png
[modify] https://crrev.com/d46daceb7047d0ab92a164a062fa26e31baae377/third_party/WebKit/LayoutTests/platform/mac/fast/forms/select/select-style-expected.png
[modify] https://crrev.com/d46daceb7047d0ab92a164a062fa26e31baae377/third_party/WebKit/LayoutTests/platform/mac/fast/gradients/conic-gradient-expected.png
[modify] https://crrev.com/d46daceb7047d0ab92a164a062fa26e31baae377/third_party/WebKit/LayoutTests/platform/mac/fast/gradients/conic-gradient-out-of-range-expected.png
[modify] https://crrev.com/d46daceb7047d0ab92a164a062fa26e31baae377/third_party/WebKit/LayoutTests/platform/mac/fast/gradients/conic-gradient-positioning-expected.png
[modify] https://crrev.com/d46daceb7047d0ab92a164a062fa26e31baae377/third_party/WebKit/LayoutTests/platform/mac/fast/gradients/crash-on-zero-radius-expected.png
[modify] https://crrev.com/d46daceb7047d0ab92a164a062fa26e31baae377/third_party/WebKit/LayoutTests/platform/mac/fast/gradients/repeating-conic-gradient-expected.png
[modify] https://crrev.com/d46daceb7047d0ab92a164a062fa26e31baae377/third_party/WebKit/LayoutTests/platform/mac/fast/gradients/simple-gradients-expected.png
[modify] https://crrev.com/d46daceb7047d0ab92a164a062fa26e31baae377/third_party/WebKit/LayoutTests/platform/mac/fast/gradients/unprefixed-linear-gradients-color-hints-expected.png
[modify] https://crrev.com/d46daceb7047d0ab92a164a062fa26e31baae377/third_party/WebKit/LayoutTests/platform/mac/fast/gradients/unprefixed-radial-gradients-color-hints-expected.png
[modify] https://crrev.com/d46daceb7047d0ab92a164a062fa26e31baae377/third_party/WebKit/LayoutTests/platform/mac/fast/gradients/unprefixed-repeating-gradient-color-hint-expected.png
[modify] https://crrev.com/d46daceb7047d0ab92a164a062fa26e31baae377/third_party/WebKit/LayoutTests/platform/mac/svg/zoom/page/zoom-background-images-expected.png
[modify] https://crrev.com/d46daceb7047d0ab92a164a062fa26e31baae377/third_party/WebKit/LayoutTests/platform/win/css3/background/background-large-position-and-size-remains-stable-expected.png
[modify] https://crrev.com/d46daceb7047d0ab92a164a062fa26e31baae377/third_party/WebKit/LayoutTests/platform/win/css3/masking/mask-repeat-round-content-expected.png
[modify] https://crrev.com/d46daceb7047d0ab92a164a062fa26e31baae377/third_party/WebKit/LayoutTests/platform/win/css3/masking/mask-repeat-space-padding-expected.png
[modify] https://crrev.com/d46daceb7047d0ab92a164a062fa26e31baae377/third_party/WebKit/LayoutTests/platform/win/fast/gradients/conic-gradient-expected.png
[modify] https://crrev.com/d46daceb7047d0ab92a164a062fa26e31baae377/third_party/WebKit/LayoutTests/platform/win/fast/gradients/conic-gradient-out-of-range-expected.png
[modify] https://crrev.com/d46daceb7047d0ab92a164a062fa26e31baae377/third_party/WebKit/LayoutTests/platform/win/fast/gradients/conic-gradient-positioning-expected.png
[modify] https://crrev.com/d46daceb7047d0ab92a164a062fa26e31baae377/third_party/WebKit/LayoutTests/platform/win/fast/gradients/crash-on-zero-radius-expected.png
[modify] https://crrev.com/d46daceb7047d0ab92a164a062fa26e31baae377/third_party/WebKit/LayoutTests/platform/win/fast/gradients/repeating-conic-gradient-expected.png
[modify] https://crrev.com/d46daceb7047d0ab92a164a062fa26e31baae377/third_party/WebKit/LayoutTests/platform/win/fast/gradients/simple-gradients-expected.png
[modify] https://crrev.com/d46daceb7047d0ab92a164a062fa26e31baae377/third_party/WebKit/LayoutTests/platform/win/fast/gradients/unprefixed-linear-gradients-color-hints-expected.png
[modify] https://crrev.com/d46daceb7047d0ab92a164a062fa26e31baae377/third_party/WebKit/LayoutTests/platform/win/fast/gradients/unprefixed-radial-gradients-color-hints-expected.png
[modify] https://crrev.com/d46daceb7047d0ab92a164a062fa26e31baae377/third_party/WebKit/LayoutTests/platform/win/fast/gradients/unprefixed-repeating-gradient-color-hint-expected.png
[modify] https://crrev.com/d46daceb7047d0ab92a164a062fa26e31baae377/third_party/WebKit/LayoutTests/platform/win/svg/zoom/page/zoom-background-images-expected.png
[modify] https://crrev.com/d46daceb7047d0ab92a164a062fa26e31baae377/third_party/WebKit/LayoutTests/svg/zoom/page/zoom-background-image-tiled-expected.png
[modify] https://crrev.com/d46daceb7047d0ab92a164a062fa26e31baae377/third_party/WebKit/Source/core/layout/LayoutBox.cpp
[modify] https://crrev.com/d46daceb7047d0ab92a164a062fa26e31baae377/third_party/WebKit/Source/core/paint/BackgroundImageGeometry.cpp
[modify] https://crrev.com/d46daceb7047d0ab92a164a062fa26e31baae377/third_party/WebKit/Source/core/paint/BackgroundImageGeometry.h
[modify] https://crrev.com/d46daceb7047d0ab92a164a062fa26e31baae377/third_party/WebKit/Source/core/paint/BoxPainterBase.cpp
[modify] https://crrev.com/d46daceb7047d0ab92a164a062fa26e31baae377/third_party/WebKit/Source/platform/geometry/LayoutRectOutsets.h

With d46daceb7047d0ab92a164a062fa26e31baae377 we are now using drawImageRect for simple/non-tiled sprites, and avoiding the texture upload limitation.

There is still the case where explicit tiling is required (https://codepen.io/fmalita/pen/ZaVNvQ?editors=1100), but that is unlikely to come up in practice in the context CSS sprites or large atlases.

hi famlita,
Please help check demo(tao.html) in https://bugs.chromium.org/p/chromium/issues/detail?id=780406
whether OK or not.
Thanks

Re #42,
Why draw background image use "background-position: 0 -440px" must drawTiledImage?

#0  SkRecorder::onDrawRect () at ../../third_party/skia/src/core/SkRecorder.cpp:144
#1  0x992faf6a in blink::Image::drawPattern () at ../../third_party/WebKit/Source/platform/graphics/Image.cpp:329
#2  0x992fb256 in blink::Image::drawTiledBackground () at ../../third_party/WebKit/Source/platform/graphics/Image.cpp:121
#3  0x992f3ed2 in blink::GraphicsContext::drawTiledImage ()
    at ../../third_party/WebKit/Source/platform/graphics/GraphicsContext.cpp:903
#4  0x99c7eda0 in blink::BoxPainter::paintFillLayer (obj=..., paintInfo=..., color=..., bgLayer=..., rect=..., 
    bleedAvoidance=bleedAvoidance@entry=blink::BackgroundBleedNone, box=box@entry=0x0, boxSize=LayoutSize(0px, 0px), 
    op=op@entry=SkBlendMode::kSrcOver, backgroundObject=backgroundObject@entry=0x0)
    at ../../third_party/WebKit/Source/core/paint/BoxPainter.cpp:829

Unfortunately tao.html from issue 780406 is not fixed.  I'm pretty sure the root cause is the same (texture too large for upload, cannot use shaders/tiling), but the trigger must be different since it doesn't use borders.  I'll investigate.

Re. c#44, are you asking specifically about https://codepen.io/fmalita/pen/ZaVNvQ?editors=1100?

Note that while your/tao.html atlas is horizontal, that example uses a vertical atlas (40x4760).  Then the div background size is 400x40 while the sprite is only 40px wide -- so it must be tiled horizontally (10x).

I see, thanks.

But i think it is hard to avoid all cases that use drawRect to draw a huge image in blink. 
The root cause it's skia gpu drawRect