New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 876349 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Dec 4
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug

Blocked on:
issue skia:8410



Sign in to add a comment

Don't use N32 for resizing F16 images on Android

Project Member Reported by zakerinasab@chromium.org, Aug 21

Issue description

In Software and Gpu ImageDecodeCache, we fallback to kLow_SkFilterQuality from kMedium_SkFilterQuality when downscaling half float backed cached images on Android. This is due to the failures observed on Android KitKat when using mip maps for down scaling.
Fix this whenever the root cause is identified and fixed in Skia.
 
Labels: OS-Android
Summary: Use medium filter quality for down scaling half float baked cached images on Android (was: Use medium filter quality for down scaling half float baked cached images)
When doing this, we also need to update the affected unit tests accordingly.
Cc: mtkl...@google.com reed@google.com
Regarding the crash in Skia, this is the simplified call sequence for one of the crashing software image decode cache tests:

SkDraw::drawPaint() -->
SkCreateRasterPipelineBlitter() -->
SkRasterPipelineBlitter::Create() -->
SkShaderBase::appendStages() -->
SkImageShader::onAppendStages() -->
SkBitmapController::RequestBitmap()

In this function, the first line that creates the state object, causes a crash.

That makes it seem like an out-of-memory issue, with the SkArenaAlloc trying (and failing) to allocate.
Test images are from 256x256 to 500x500.
Status: WontFix (was: Assigned)
It was decided to stick to medium filter quality and decode to 8888 instead of F16 wherever necessary to address this issue. Since those devices (Android KitKat and lower) do not use high bit depth displays, this should be fine (we eventually may tie color managed canvas to the presence of a wide gamut display). Closing this bug though.
Blockedon: skia:8410
Status: Assigned (was: WontFix)
Summary: Unify scaling code path in Software and GPU ImageDecodeCache for Android KitKat and others (was: Use medium filter quality for down scaling half float baked cached images on Android)
After more discussion with khushalsagar@, we though it's better to file a Skia bug so we can keep track of this and unify the code path if this gets fixed. skia:8410.
Owner: ----
Status: Available (was: Assigned)
Hey there, I noticed that a bunch of cc_unittests started failing on Android release builds after https://chromium.googlesource.com/chromium/src/+/9afecdff864427ce00a8da161880b53d8e0a54f9 landed.

I'm not very familiar with the src/cc code but I found that if we restrict resizing F16 images on Android to just low quality and below (regardless of Android version) then the tests are able to pass.

Proposed change:
https://chromium-review.googlesource.com/c/chromium/src/+/1357557

Without this, running the cc_unittests on a stock Pixel 2 running Android P results in 10 failing tests due to crashes:

out/Release/bin/run_cc_unittests --num_retries=1
C  219.959s Main  ********************************************************************************
C  219.959s Main  Detailed Logs
C  219.960s Main  ********************************************************************************
C  219.962s Main  [CRASH] GpuImageDecodeCacheTests/GpuImageDecodeCacheTest.CacheDecodesExpectedFrames/2:
C  219.962s Main  [ RUN      ] GpuImageDecodeCacheTests/GpuImageDecodeCacheTest.CacheDecodesExpectedFrames/2
C  219.962s Main  [ CRASHED      ]
C  219.962s Main  
C  219.962s Main  [CRASH] GpuImageDecodeCacheTests/GpuImageDecodeCacheTest.CacheDecodesExpectedFrames/5:
C  219.962s Main  [ RUN      ] GpuImageDecodeCacheTests/GpuImageDecodeCacheTest.CacheDecodesExpectedFrames/5
C  219.962s Main  [ CRASHED      ]
C  219.962s Main  
C  219.962s Main  [CRASH] GpuImageDecodeCacheTests/GpuImageDecodeCacheTest.GetLargeScaledDecodedImageForDraw/2:
C  219.962s Main  [ RUN      ] GpuImageDecodeCacheTests/GpuImageDecodeCacheTest.GetLargeScaledDecodedImageForDraw/2
C  219.962s Main  [ CRASHED      ]
C  219.962s Main  
C  219.963s Main  [CRASH] GpuImageDecodeCacheTests/GpuImageDecodeCacheTest.GetLargeScaledDecodedImageForDraw/5:
C  219.963s Main  [ RUN      ] GpuImageDecodeCacheTests/GpuImageDecodeCacheTest.GetLargeScaledDecodedImageForDraw/5
C  219.963s Main  [ CRASHED      ]
C  219.963s Main  
C  219.963s Main  [CRASH] GpuImageDecodeCacheTests/GpuImageDecodeCacheTest.KeepOnlyLast2ContentIds/2:
C  219.963s Main  [ RUN      ] GpuImageDecodeCacheTests/GpuImageDecodeCacheTest.KeepOnlyLast2ContentIds/2
C  219.963s Main  [ CRASHED      ]
C  219.963s Main  
C  219.963s Main  [CRASH] GpuImageDecodeCacheTests/GpuImageDecodeCacheTest.KeepOnlyLast2ContentIds/5:
C  219.963s Main  [ RUN      ] GpuImageDecodeCacheTests/GpuImageDecodeCacheTest.KeepOnlyLast2ContentIds/5
C  219.963s Main  [ CRASHED      ]
C  219.963s Main  
C  219.963s Main  [CRASH] GpuImageDecodeCacheTests/GpuImageDecodeCacheTest.NonLazyImageUploadDownscaled/2:
C  219.963s Main  [ RUN      ] GpuImageDecodeCacheTests/GpuImageDecodeCacheTest.NonLazyImageUploadDownscaled/2
C  219.963s Main  [ CRASHED      ]
C  219.963s Main  
C  219.963s Main  [CRASH] GpuImageDecodeCacheTests/GpuImageDecodeCacheTest.NonLazyImageUploadDownscaled/5:
C  219.963s Main  [ RUN      ] GpuImageDecodeCacheTests/GpuImageDecodeCacheTest.NonLazyImageUploadDownscaled/5
C  219.963s Main  [ CRASHED      ]
C  219.963s Main  
C  219.963s Main  [CRASH] SoftwareImageDecodeCacheTest_F16_ExoticColorSpace.UseClosestAvailableDecode_F16_ExoticColorSpace:
C  219.963s Main  [ RUN      ] SoftwareImageDecodeCacheTest_F16_ExoticColorSpace.UseClosestAvailableDecode_F16_ExoticColorSpace
C  219.963s Main  [ CRASHED      ]
C  219.963s Main  
C  219.963s Main  [CRASH] SoftwareImageDecodeCacheTest_F16_WideGamutCanvasColorSpace.UseClosestAvailableDecode_F16_WideGamutCanvasColorSpace:
C  219.963s Main  [ RUN      ] SoftwareImageDecodeCacheTest_F16_WideGamutCanvasColorSpace.UseClosestAvailableDecode_F16_WideGamutCanvasColorSpace
C  219.963s Main  [ CRASHED      ]
C  219.963s Main  ********************************************************************************
C  219.963s Main  Summary
C  219.963s Main  ********************************************************************************
C  219.968s Main  [==========] 3161 tests ran.
C  219.969s Main  [  PASSED  ] 3151 tests.
C  219.969s Main  [  FAILED  ] 10 tests, listed below:
C  219.969s Main  [  FAILED  ] GpuImageDecodeCacheTests/GpuImageDecodeCacheTest.CacheDecodesExpectedFrames/2 (CRASHED)
C  219.969s Main  [  FAILED  ] GpuImageDecodeCacheTests/GpuImageDecodeCacheTest.CacheDecodesExpectedFrames/5 (CRASHED)
C  219.969s Main  [  FAILED  ] GpuImageDecodeCacheTests/GpuImageDecodeCacheTest.GetLargeScaledDecodedImageForDraw/2 (CRASHED)
C  219.969s Main  [  FAILED  ] GpuImageDecodeCacheTests/GpuImageDecodeCacheTest.GetLargeScaledDecodedImageForDraw/5 (CRASHED)
C  219.969s Main  [  FAILED  ] GpuImageDecodeCacheTests/GpuImageDecodeCacheTest.KeepOnlyLast2ContentIds/2 (CRASHED)
C  219.969s Main  [  FAILED  ] GpuImageDecodeCacheTests/GpuImageDecodeCacheTest.KeepOnlyLast2ContentIds/5 (CRASHED)
C  219.969s Main  [  FAILED  ] GpuImageDecodeCacheTests/GpuImageDecodeCacheTest.NonLazyImageUploadDownscaled/2 (CRASHED)
C  219.969s Main  [  FAILED  ] GpuImageDecodeCacheTests/GpuImageDecodeCacheTest.NonLazyImageUploadDownscaled/5 (CRASHED)
C  219.969s Main  [  FAILED  ] SoftwareImageDecodeCacheTest_F16_ExoticColorSpace.UseClosestAvailableDecode_F16_ExoticColorSpace (CRASHED)
C  219.969s Main  [  FAILED  ] SoftwareImageDecodeCacheTest_F16_WideGamutCanvasColorSpace.UseClosestAvailableDecode_F16_WideGamutCanvasColorSpace (CRASHED)
C  219.969s Main  
C  219.969s Main  10 FAILED TESTS
C  219.969s Main  ********************************************************************************

With the proposed change, all cc_unittests pass:

out/Release/bin/run_cc_unittests --num_retries=1
C  149.833s Main  ********************************************************************************
C  149.833s Main  Summary
C  149.833s Main  ********************************************************************************
C  149.836s Main  [==========] 3164 tests ran.
C  149.836s Main  [  PASSED  ] 3164 tests.
C  149.836s Main  ********************************************************************************
Labels: -Pri-3 Pri-2
Summary: Don't use N32 for resizing F16 images on Android (was: Unify scaling code path in Software and GPU ImageDecodeCache for Android KitKat and others)
As reported in comment 10, issues are also reported on Android O.
Owner: brianosman@google.com
Status: Assigned (was: Available)
Project Member

Comment 14 by bugdroid1@chromium.org, Dec 4

The following revision refers to this bug:
  https://skia.googlesource.com/skia/+/b237f8ac262a284003769f93a47f7d77c3cff737

commit b237f8ac262a284003769f93a47f7d77c3cff737
Author: Brian Osman <brianosman@google.com>
Date: Tue Dec 04 15:48:11 2018

Ensure that SkMipMap pixel data is always 8 byte aligned (for F16)

Bug:  skia:8410   chromium:876349 
Change-Id: Ib97c82cd165f7e2f2e94c65fc307220b99053df3
Reviewed-on: https://skia-review.googlesource.com/c/174065
Commit-Queue: Brian Osman <brianosman@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>

[modify] https://crrev.com/b237f8ac262a284003769f93a47f7d77c3cff737/src/core/SkMipMap.cpp
[modify] https://crrev.com/b237f8ac262a284003769f93a47f7d77c3cff737/src/core/SkMipMap.h
[modify] https://crrev.com/b237f8ac262a284003769f93a47f7d77c3cff737/tests/MipMapTest.cpp

Status: Fixed (was: Assigned)

Sign in to add a comment