Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

caskbench text test case in lags heavily in skia #8

Open
chevalam opened this issue May 14, 2014 · 14 comments
Open

caskbench text test case in lags heavily in skia #8

chevalam opened this issue May 14, 2014 · 14 comments
Assignees

Comments

@chevalam
Copy link
Contributor

Output in cairo and skia text test case match correctly but skia-text test case is lags when compared to cairo text

@chevalam
Copy link
Contributor Author

Hi Henry ,

I had worked on skia-text test case and had raised issue in github for the same. Below is the call flow for drawText.
drawText() - > GrBitmapTextContext::drawText() -> drawPackedGlyph() -> drawPath().

In the above call flow , there are two stages where finish()/flush() is invoked , one in drawPackedGlyph () and one in drawText() after drawPackedGlyph().
I have commented the finish()/flush() in drawPackedGlyph() and the output for text test case is same but the benchmark numbers are better .
Would it be acceptable if we can get rid of the flush in drawPackedGlyph since there is one flush in drawText after drawPackedGlyph.

Below are the readings captured on PC and device with and without modifications.


PC/mesa 10.0 :
On commenting flush:
cairo-text 64 PASS 10 0.035369 0.081840
skia-text 64 PASS 10 0.940362 0.960749 -1073.94%

Current:
cairo-text 64 PASS 10 0.036002 0.083150
skia-text 64 PASS 10 1.542647 1.588705 -1810.65%

Below is the commented flush from GrBitmapTextContext.cpp
@@ -531,8 +531,8 @@ void GrBitmapTextContext::drawPackedGlyph(GrGlyph::PackedID packed,
}

  •    this->flushGlyphs();
    
  •    fContext->flush();
    
  • // this->flushGlyphs();
  • // fContext->flush();

Call control goes from drawPackedGlyph() to drawPath where path is drawn into stencil buffer.

Regards,
Srikanth.

@chevalam
Copy link
Contributor Author

chevalam commented Jun 4, 2014

Hi Henry ,

I have found two cases for the drawText scenario :

1.LCD Rendering - setLCDRendering(true) : ‘drawText’ takes 830 µs, the ‘drawPackedGlyph’ takes around 50 µs and there is a while loop across drawPackedGlyph i.e, ‘while(text <stop) { drawPackedGlyph() }’ which takes 812 µs.
On profiling drawPackedGlyph() , there is a time taking ‘GrTextStrike::addGlyphToAtlas()’ which invokes ‘SkGrFontScaler::getPackedGlyphImage()’ (~30µs) and ‘GrAtlasMgr::addToAtlas()(~20µs)’ .

Below Is the equation to sum the profiling breakup :
drawText - 830 µs = while{ drawPackedGlyph () [50 µs] } - 812 µs drawPackedGlyph () [50 µs] = getPackedGlyphImage() [30 µs]+ GrAtlasMgr::addToAtlas() [20 µs]

I am currently working to verify if either of the time consuming areas i.e., the while loop , getPackedGlyphImage or addToAtlas can be optimized .

  1. Default Rendering :
    drawText is redirected to drawPath via drawPackedGlyph in default rendering scenario .
    But the issue I observed here is that unlike in LCD Rendering , a flush() is called for every drawPackedGlyph () . So if the while across drawPackedGlyph loops for 10 times , flush is called 10 times .
    This seems to be an issue that might be optimized . I am currently checking on why the flush is invoked so frequently in the case of default rendering which might help rendering times.

Regards,
Srikanth.

@chevalam
Copy link
Contributor Author

chevalam commented Jun 4, 2014

Hi Henry ,

Follow up on drawText observations :

a) Default Rendering :
drawPath is used to handle default rendering scenario . The path followed for LCD Rendering i.e., addToAtlas , getGlyphMatrics is also traversed for default rendering. But textures won’t be used for default rendering scenario , fplot is returned NULL and addToAtlas() , freeUnusedPlot() routines need not be invoked for default rendering scenario . Below are the readings captured with and without the texture code for default rendering.

Below are the readings before and after modifications. It is observed that there is an improvement in skia readings.

Before:
cairo-text 100 PASS 100 5
skia-text 100 PASS 100 0 -1094.91%

After:
cairo-text 100 PASS 100 5
skia-text 100 PASS 100 1 -491.86%

I am trying to find out the flag to differentiate between default rendering and lcd rendering handling. Kindly let me know if the approach is acceptable . Below is the patch for the same:


diff --git a/src/gpu/GrBitmapTextContext.cpp b/src/gpu/GrBitmapTextContext.cpp index 3b5e27a..a1e18c4 100755
--- a/src/gpu/GrBitmapTextContext.cpp
+++ b/src/gpu/GrBitmapTextContext.cpp
@@ -514,6 +514,8 @@ void GrBitmapTextContext::drawPackedGlyph(GrGlyph::PackedID packed,
}

 if (NULL == glyph->fPlot) {
  • if (fSkPaint.isLCDRenderText()) 
    
  •   {
     if (fStrike->addGlyphToAtlas(glyph, scaler)) {
         goto HAS_ATLAS;
     }
    

    @@ -539,7 +541,7 @@ void GrBitmapTextContext::drawPackedGlyph(GrGlyph::PackedID packed,
    fStrike->addGlyphToAtlas(glyph, scaler)) {
    goto HAS_ATLAS;
    }

  •   }
     if (NULL == glyph->fPath) {
         SkPath\* path = SkNEW(SkPath);
         if (!scaler->getGlyphPath(glyph->glyphID(), path)) {
    

b) LCD Rendering :
drawText executes drawPackedGlyph in a while loop . Each drawPackedGlyph() invokes two routines which majorly contribute to the time taken by drawText.
getGlyphMatrics – retrives glyph through hash table retrieval of glyphID and copies the glyph data into image buffer addToAtlas – Invokes BindTexture,glTexSubImage2D and copies uses image buffer for the texture It would help if the while loop could be replaced by a single call instead of multiple drawPackedGlyph calls which invoke 1 and 2 .

Regards,
Srikanth.

elima pushed a commit that referenced this issue Jun 10, 2014
TSAN shows us racing on the function pointers.  Might as well fix it.

WARNING: ThreadSanitizer: data race (pid=19995)
  Read of size 8 at 0x7f703affb048 by thread T12 (mutexes: write M2957):
    #0 SkBitmap::internalErase(SkIRect const&, unsigned int, unsigned int, unsigned int, unsigned int) const /var/scratch/Release/../../../usr/local/google/home/mtklein/skia/src/core/SkBitmap.cpp:886 (tests+0x0000003511ca)
    #1 SkBitmap::eraseARGB(unsigned int, unsigned int, unsigned int, unsigned int) const /var/scratch/Release/../../../usr/local/google/home/mtklein/skia/src/core/SkBitmap.cpp:919 (tests+0x0000003534bf)
    #2 (anonymous namespace)::DecodingImageGenerator::getPixels(SkImageInfo const&, void*, unsigned long) /var/scratch/Release/../../../usr/local/google/home/mtklein/skia/src/images/SkDecodingImageGenerator.cpp:195 (tests+0x00000051bee1)
    #3 SkDiscardablePixelRef::onNewLockPixels(SkPixelRef::LockRec*) /var/scratch/Release/../../../usr/local/google/home/mtklein/skia/src/lazy/SkDiscardablePixelRef.cpp:63 (tests+0x00000039ad9c)
    #4 SkPixelRef::lockPixels(SkPixelRef::LockRec*) /var/scratch/Release/../../../usr/local/google/home/mtklein/skia/src/core/SkPixelRef.cpp:179 (tests+0x0000003fec23)
    #5 SkBitmap::lockPixels() const /var/scratch/Release/../../../usr/local/google/home/mtklein/skia/src/core/SkBitmap.cpp:414 (tests+0x00000034e41e)
    #6 SkAutoLockPixels /var/scratch/Release/../../../usr/local/google/home/mtklein/skia/include/core/SkBitmap.h:819 (tests+0x0000002752f3)
    #7 ImageDecoderOptions(skiatest::Reporter*) /var/scratch/Release/../../../usr/local/google/home/mtklein/skia/tests/ImageDecodingTest.cpp:565 (tests+0x000000275d03)
    #8 skiatest::Test::run() /var/scratch/Release/../../../usr/local/google/home/mtklein/skia/tests/Test.cpp:107 (tests+0x0000002263e7)
    #9 SkTestRunnable::run() /var/scratch/Release/../../../usr/local/google/home/mtklein/skia/tests/skia_test.cpp:108 (tests+0x0000001d8607)
    #10 SkThreadPoolPrivate::ThreadLocal<void>::run(SkTRunnable<void>*) /var/scratch/Release/../../../usr/local/google/home/mtklein/skia/include/utils/SkThreadPool.h:108 (tests+0x0000001d817e)
    #11 thread_start(void*) /var/scratch/Release/../../../usr/local/google/home/mtklein/skia/src/utils/SkThreadUtils_pthread.cpp:66 (tests+0x000000604347)

  Previous write of size 8 at 0x7f703affb048 by thread T26:
    [failed to restore the stack]


BUG=skia:1792
R=bungeman@google.com, mtklein@google.com, reed@google.com

Author: mtklein@chromium.org

Review URL: https://codereview.chromium.org/250503003

git-svn-id: http://skia.googlecode.com/svn/trunk@14548 2bbb7eff-a529-9590-31e7-b0007b416f81
@chevalam
Copy link
Contributor Author

Hi Henry ,

In continuation of the earlier analysis of drawText , I have observed that there are other drawText() APIs like drawTextOnPath(), drawTextOnPos() which also call render/call drawPath() letter by letter in a while loop .

In the case of drawText() , SkUTF8_NextUnichar() retrieves hash ID for each glyph on which drawPath() is called . This is repeated for all the letters passed on to drawText.

Would it help if we can add all the letters to the path and call drawPath only once instead of multiple drawPaths ?

I am trying to check as to how the text scenario is being handled cairo ?

Regards,
Srikanth.


Hi Henry ,
LCD Rendering - setLCDRenderText(true) is invoked before drawText() is called. Texture rendering is used to draw the text onto the screen in this scenario .
Default Rendering - drawText() is used to render text onto screen . drawText is redirected to drawPath via drawPackedGlyph in this scenario .

In both the cases i.e, texture rendering / drawPath , it seems that the rendering is being done letter by letter . I am currently checking if this can be improved .

Regards,
Srikanth.


Hi, Srikanth

I don't have enough knowledge on difference of LCDrendering and default rendering in skia. What is the difference?

Thanks
Henry

@henrysong
Copy link
Contributor

Hi, Srikanth

First We need to understand difference of LCDrendering vs. default text rendering.

Second, we need to know which one is used by Chromium

Third, it is not important drawPath() is called for each glyph. It is important whether each drawPath results in a recordDraw() or all drawPath() are batched. If each drawPath() results in a recordDraw(), then it will be slow, but if drawPath() does not results in a recordDraw() then it means draw text are batched. According to flashcanvas.net test, chromium draw text is faster than cairo. In cairo, draw texts are batched.

Thanks

Henry


From: Srikanth Chevalam [notifications@github.com]
Sent: Tuesday, June 10, 2014 11:01 PM
To: Samsung/skia
Subject: Re: [skia] caskbench text test case in lags heavily in skia (#8)

Hi Henry ,

In continuation of the earlier analysis of drawText , I have observed that there are other drawText() APIs like drawTextOnPath(), drawTextOnPos() which also call render/call drawPath() letter by letter in a while loop .

In the case of drawText() , SkUTF8_NextUnichar() retrieves hash ID for each glyph on which drawPath() is called . This is repeated for all the letters passed on to drawText.

Would it help if we can add all the letters to the path and call drawPath only once instead of multiple drawPaths ?

I am trying to check as to how the text scenario is being handled cairo ?

Regards,
Srikanth.


Hi Henry ,
LCD Rendering - setLCDRenderText(true) is invoked before drawText() is called. Texture rendering is used to draw the text onto the screen in this scenario .
Default Rendering - drawText() is used to render text onto screen . drawText is redirected to drawPath via drawPackedGlyph in this scenario .

In both the cases i.e, texture rendering / drawPath , it seems that the rendering is being done letter by letter . I am currently checking if this can be improved .

Regards,
Srikanth.


Hi, Srikanth

I don't have enough knowledge on difference of LCDrendering and default rendering in skia. What is the difference?

Thanks
Henry


Reply to this email directly or view it on GitHubhttps://github.com//issues/8#issuecomment-45704962.

@chevalam chevalam self-assigned this Jun 17, 2014
@chevalam
Copy link
Contributor Author

Hi Henry ,

Thank you for your comments. Some findings based on your inputs.
1)Chromium uses LCDRendering as default since SkiaTextRenderer() of render_text.cc has paint_.setLCDRenderText(true);
2) It is observed that each drawPath() results in a recordDraw() . So drawPath() doesnot seem to be batched.
3)LCDRendering uses BindTexture & glTexSubImage2D and copies using image buffer for texture . Default rendering uses drawPath to draw text .

Regards,
Srikanth

@chevalam
Copy link
Contributor Author

Hi Henry ,

I have ported and executed text test case of flashcanvas.net on linux and observed that skia text (30fps)is slower than cairo text (60fps) . I guess the chromium flashcanvas.net might be running on skia image backend

Regards,
Srikanth.

@chevalam
Copy link
Contributor Author

Hi Henry,

Some more observations on the skia drawText performance analysis:
LCD Rendering :

  1. When a random color is used in caskbench text case, skia lags when compared to cairo . Reason is that the number of glyph cache misses is more and all the glyphs are created newly and inserted in fcache.
  2. When a fixed color is used in caskbench text case , no lag is observed in skia compared to cairo . Reason is that number of glyph cache hits is more and all the glyphs are retrieved from fcache.

Default Rendering.

  1. Glyph Cache search always misses and there is no effect of random or fixed color on skia's performance . We are currently debugging as to why glyphs are not found cache even for fixed color.

Regards,
Srikanth.

@chevalam
Copy link
Contributor Author

Hi Henry ,

Please find some clarifications on the earlier findings on drawText rendering
Why Default Rendering for drawText goes via drawPath -
fTextureRedSupport is set to TRUE in skia because of which TexImage2D is attempted with internalFormat=GL_RED . TexImage2D fails for GL_RED , texture is not created and rendering is completed with drawPath and is very slow when compared to cairo.

For testing purpose , I have manually set fTextureRedSupport to FALSE in GrGLCaps.cpp . This sets internalFormat=GL_ALPHA , TexImage2D passes for GL_ALPHA and texture is created and performance of skia text case for default rendering is now on par with cairo draw Text.

Below are the readings
fTextureRedSupport = 1(Default)
./src/caskbench -t egl text
cairo-text 100 PASS 100 9
skia-text 100 PASS 100 0 -2332.24%

fTextureRedSupport = 0(Modified)
./src/caskbench -t egl text
cairo-text 100 PASS 100 9
skia-text 100 PASS 100 10 3.63%

Could you kindly give your inputs on how to go about the above scenario.

Regards,
Srikanth.

@henrysong
Copy link
Contributor

Hi, Srikanth

That is good finding. I have not looked into text rendering part yet. I would like to know

  1. What is chromium using - LCD rendering or default rendering
  2. Why is skia using GL_RED?

Thanks

Henry


From: Srikanth Chevalam [notifications@github.com]
Sent: Thursday, June 26, 2014 5:07 AM
To: Samsung/skia
Cc: Henry (Yu) Song - SISA
Subject: Re: [skia] caskbench text test case in lags heavily in skia (#8)

Hi Henry ,

Please find some clarifications on the earlier findings on drawText rendering
Why Default Rendering for drawText goes via drawPath -
fTextureRedSupport is set to TRUE in skia because of which TexImage2D is attempted with internalFormat=GL_RED . TexImage2D fails for GL_RED , texture is not created and rendering is completed with drawPath and is very slow when compared to cairo.

For testing purpose , I have manually set fTextureRedSupport to FALSE in GrGLCaps.cpp . This sets internalFormat=GL_ALPHA , TexImage2D passes for GL_ALPHA and texture is created and performance of skia text case for default rendering is now on par with cairo draw Text.

Below are the readings
fTextureRedSupport = 1(Default)
./src/caskbench -t egl text
cairo-text 100 PASS 100 9
skia-text 100 PASS 100 0 -2332.24%

fTextureRedSupport = 0(Modified)
./src/caskbench -t egl text
cairo-text 100 PASS 100 9
skia-text 100 PASS 100 10 3.63%

Could you kindly give your inputs on how to go about the above scenario.

Regards,
Srikanth.


Reply to this email directly or view it on GitHubhttps://github.com//issues/8#issuecomment-47217629.

@elima
Copy link

elima commented Jun 26, 2014

  1. When a random color is used in caskbench text case, skia lags when compared to cairo . Reason is that the number of glyph cache misses is more and all the glyphs are created newly and inserted in fcache.
  2. When a fixed color is used in caskbench text case , no lag is observed in skia compared to cairo . Reason is that number of glyph cache hits is more and all the glyphs are retrieved from fcache.

I wonder if an earlier problem is that cached glyphs have the color attached. I thought it was just alpha level since that's what FreeType2 gives in normal rendering. Shouldn't color be passed separately and multiplied with the alpha from the glyph's tex cache?

@henrysong
Copy link
Contributor

Right, in cairo, only glyphs are cached in alpha-only texture. I don't see any reason to attach color.
I would imagine skia should do the same.

Thanks

Henry


From: Eduardo Lima Mitev [notifications@github.com]
Sent: Thursday, June 26, 2014 8:20 AM
To: Samsung/skia
Cc: Henry (Yu) Song - SISA
Subject: Re: [skia] caskbench text test case in lags heavily in skia (#8)

  1. When a random color is used in caskbench text case, skia lags when compared to cairo . Reason is that the number of glyph cache misses is more and all the glyphs are created newly and inserted in fcache.
  2. When a fixed color is used in caskbench text case , no lag is observed in skia compared to cairo . Reason is that number of glyph cache hits is more and all the glyphs are retrieved from fcache.

I wonder if an earlier problem is that cached glyphs have the color attached. I thought it was just alpha level since that's what FreeType2 gives in normal rendering. Shouldn't color be passed separately and multiplied with the alpha from the glyph's tex cache?


Reply to this email directly or view it on GitHubhttps://github.com//issues/8#issuecomment-47239439.

@chevalam
Copy link
Contributor Author

Hi Henry,

  1. What is chromium using - LCD rendering or default rendering
  2. Why is skia using GL_RED?

1.Chromium is using LCD Rendering . Reference src/ui/gfx/render_text.cc where setLCDRenderText is set to true by default.
2. skia uses GL_RED if GL version > 3.0 or if GL_EXT_texture_rg otherwise GL_ALPHA is used for teximage2d.

Regards,
Srikanth.

@chevalam
Copy link
Contributor Author

chevalam commented Jul 4, 2014

Hi Henry ,

Right, in cairo, only glyphs are cached in alpha-only texture. I don't see any reason to attach color.
I would imagine skia should do the same.

Some observations on LCD Rendering performance lag in drawText :
There is a setLuminanceColor(computeLuminanceColor(paint)); in skia paint where font color is being used used in cache handling . Disabling this routine or using setLuminanceColor(0) prevents color from being used in cache and skia readings are on par with cairo .

Current reading:
cairo-text 100 PASS 100 383
skia-text 100 PASS 100 31 -1142.23%

Modified reading after disabling setLuminanceColor /setLuminanceColor(0) :
cairo-text 100 PASS 100 390
skia-text 100 PASS 100 419 6.99%

We are currently investigating on the effects of setLuminanceColor(0) on SkPaint.

It is also observed that setLuminanceColor(0) gives good performance on skia image backend for drawText case

Regards,
Srikanth

suyambulingamrm pushed a commit that referenced this issue May 15, 2015
…ch (patchset #10 id:200001 of https://codereview.chromium.org/729463002/)

Reason for revert:
revert it due to a memory leak.
==8017==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 25992 byte(s) in 2 object(s) allocated from:
    #0 0x7feb53030e0b in __interceptor_malloc
(/home/default/storage/skia-repo/buildbot/skiabot-linux-xsan-000/build/slave/Test-Ubuntu13_10-GCE-NoGPU-x86_64-Debug-ASAN/build/skia/out/Debug/gm+0x24fe0b)
    #1 0x7feb54d54f76 in sk_malloc_flags(unsigned long, unsigned int)
/home/default/storage/skia-repo/buildbot/skiabot-linux-xsan-000/build/slave/Test-Ubuntu13_10-GCE-NoGPU-x86_64-Debug-ASAN/build/skia/out/Debug/../../src/ports/SkMemory_malloc.cpp:54:15
    #2 0x7feb54d54d4a in sk_malloc_throw(unsigned long)
/home/default/storage/skia-repo/buildbot/skiabot-linux-xsan-000/build/slave/Test-Ubuntu13_10-GCE-NoGPU-x86_64-Debug-ASAN/build/skia/out/Debug/../../src/ports/SkMemory_malloc.cpp:40:12
    #3 0x7feb539f5a77 in SkMask::AllocImage(unsigned long)
/home/default/storage/skia-repo/buildbot/skiabot-linux-xsan-000/build/slave/Test-Ubuntu13_10-GCE-NoGPU-x86_64-Debug-ASAN/build/skia/out/Debug/../../src/core/SkMask.cpp:37:22
    #4 0x7feb53fe5c34 in (anonymous
namespace)::copy_cacheddata_to_mask(SkCachedData*, SkMask*)
/home/default/storage/skia-repo/buildbot/skiabot-linux-xsan-000/build/slave/Test-Ubuntu13_10-GCE-NoGPU-x86_64-Debug-ASAN/build/skia/out/Debug/../../src/core/SkMaskCache.cpp:25:20
    #5 0x7feb53fea064 in SkMaskCache::FindAndCopy(float, SkBlurStyle,
SkBlurQuality, SkRect const*, int, SkMask*)
/home/default/storage/skia-repo/buildbot/skiabot-linux-xsan-000/build/slave/Test-Ubuntu13_10-GCE-NoGPU-x86_64-Debug-ASAN/build/skia/out/Debug/../../src/core/SkMaskCache.cpp:224:26
    #6 0x7feb539f957e in SkMaskFilter::filterPath(SkPath const&, SkMatrix
const&, SkRasterClip const&, SkBlitter*, SkPaint::Style) const
/home/default/storage/skia-repo/buildbot/skiabot-linux-xsan-000/build/slave/Test-Ubuntu13_10-GCE-NoGPU-x86_64-Debug-ASAN/build/skia/out/Debug/../../src/core/SkMaskFilter.cpp:270:14
    #7 0x7feb5392e920 in SkDraw::drawPath(SkPath const&, SkPaint const&,
SkMatrix const*, bool, bool, SkBlitter*) const
/home/default/storage/skia-repo/buildbot/skiabot-linux-xsan-000/build/slave/Test-Ubuntu13_10-GCE-NoGPU-x86_64-Debug-ASAN/build/skia/out/Debug/../../src/core/SkDraw.cpp:1117:13
    #8 0x7feb53694afc in SkDraw::drawPath(SkPath const&, SkPaint const&,
SkMatrix const*, bool) const
/home/default/storage/skia-repo/buildbot/skiabot-linux-xsan-000/build/slave/Test-Ubuntu13_10-GCE-NoGPU-x86_64-Debug-ASAN/build/skia/out/Debug/../../include/core/SkDraw.h:54:9
    #9 0x7feb5368d799 in SkBitmapDevice::drawPath(SkDraw const&, SkPath const&,
SkPaint const&, SkMatrix const*, bool)
/home/default/storage/skia-repo/buildbot/skiabot-linux-xsan-000/build/slave/Test-Ubuntu13_10-GCE-NoGPU-x86_64-Debug-ASAN/build/skia/out/Debug/../../src/core/SkBitmapDevice.cpp:216:5
    #10 0x7feb5386aa57 in SkCanvas::onDrawPath(SkPath const&, SkPaint const&)
/home/default/storage/skia-repo/buildbot/skiabot-linux-xsan-000/build/slave/Test-Ubuntu13_10-GCE-NoGPU-x86_64-Debug-ASAN/build/skia/out/Debug/../../src/core/SkCanvas.cpp:1908:9
    #11 0x7feb5386386b in SkCanvas::drawPath(SkPath const&, SkPaint const&)
/home/default/storage/skia-repo/buildbot/skiabot-linux-xsan-000/build/slave/Test-Ubuntu13_10-GCE-NoGPU-x86_64-Debug-ASAN/build/skia/out/Debug/../../src/core/SkCanvas.cpp:1703:5
    #12 0x7feb53109572 in Blur2RectsNonNinePatchGM::onDraw(SkCanvas*)
/home/default/storage/skia-repo/buildbot/skiabot-linux-xsan-000/build/slave/Test-Ubuntu13_10-GCE-NoGPU-x86_64-Debug-ASAN/build/skia/out/Debug/../../gm/blurs.cpp:166:9

Original issue's description:
> Cache blur mask for rects which can not break into nine-patch
>
> With this CL performance improves:
> blurrectsnonninepatch   42.4us -> 20.5us        0.48x
>
> BUG=431021,skia:3118
>
> Committed: https://skia.googlesource.com/skia/+/5a5b4e900ee69540fc35952882a323c63d6d0db9

TBR=reed@google.com,humper@google.com,reed@chromium.org
NOTREECHECKS=true
NOTRY=true
BUG=431021,skia:3118

Review URL: https://codereview.chromium.org/844673002
suyambulingamrm pushed a commit that referenced this issue May 15, 2015
…odereview.chromium.org/849173004/)

Reason for revert:
==31530==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 224 byte(s) in 2 object(s) allocated from:
    #0 0x7febb223f65b in operator new(unsigned long) (/home/default/storage/skia-repo/buildbot/skiabot-linux-xsan-001/build/slave/Test-Ubuntu13_10-GCE-NoGPU-x86_64-Debug-ASAN/build/skia/out/Debug/dm+0x35865b)
    #1 0x7febb3483c56 in SkMipMap::Build(SkBitmap const&, SkDiscardableMemory* (*)(unsigned long)) /home/default/storage/skia-repo/buildbot/skiabot-linux-xsan-001/build/slave/Test-Ubuntu13_10-GCE-NoGPU-x86_64-Debug-ASAN/build/skia/out/Debug/../../src/core/SkMipMap.cpp:181:9
    #2 0x7febb2c37914 in ShowMipLevels::onDraw(SkCanvas*) /home/default/storage/skia-repo/buildbot/skiabot-linux-xsan-001/build/slave/Test-Ubuntu13_10-GCE-NoGPU-x86_64-Debug-ASAN/build/skia/out/Debug/../../gm/downsamplebitmap.cpp:228:24
    #3 0x7febb2295736 in skiagm::GM::drawContent(SkCanvas*) /home/default/storage/skia-repo/buildbot/skiabot-linux-xsan-001/build/slave/Test-Ubuntu13_10-GCE-NoGPU-x86_64-Debug-ASAN/build/skia/out/Debug/../../gm/gm.cpp:32:5
    #4 0x7febb2295019 in skiagm::GM::draw(SkCanvas*) /home/default/storage/skia-repo/buildbot/skiabot-linux-xsan-001/build/slave/Test-Ubuntu13_10-GCE-NoGPU-x86_64-Debug-ASAN/build/skia/out/Debug/../../gm/gm.cpp:24:5
    #5 0x7febb226722f in DM::GMSrc::draw(SkCanvas*) const /home/default/storage/skia-repo/buildbot/skiabot-linux-xsan-001/build/slave/Test-Ubuntu13_10-GCE-NoGPU-x86_64-Debug-ASAN/build/skia/out/Debug/../../dm/DMSrcSink.cpp:22:5
    #6 0x7febb226d929 in DM::RasterSink::draw(DM::Src const&, SkBitmap*, SkWStream*) const /home/default/storage/skia-repo/buildbot/skiabot-linux-xsan-001/build/slave/Test-Ubuntu13_10-GCE-NoGPU-x86_64-Debug-ASAN/build/skia/out/Debug/../../dm/DMSrcSink.cpp:230:12
    #7 0x7febb224e004 in Task::Run(Task*) /home/default/storage/skia-repo/buildbot/skiabot-linux-xsan-001/build/slave/Test-Ubuntu13_10-GCE-NoGPU-x86_64-Debug-ASAN/build/skia/out/Debug/../../dm/DM.cpp:254:25
    #8 0x7febb3805e4c in (anonymous namespace)::ThreadPool::Loop(void*) /home/default/storage/skia-repo/buildbot/skiabot-linux-xsan-001/build/slave/Test-Ubuntu13_10-GCE-NoGPU-x86_64-Debug-ASAN/build/skia/out/Debug/../../src/core/SkTaskGroup.cpp:149:13
    #9 0x7febb460dc7c in thread_start(void*) /home/default/storage/skia-repo/buildbot/skiabot-linux-xsan-001/build/slave/Test-Ubuntu13_10-GCE-NoGPU-x86_64-Debug-ASAN/build/skia/out/Debug/../../src/utils/SkThreadUtils_pthread.cpp:66:9
    #10 0x7febb1251f6d in start_thread /build/buildd/eglibc-2.17/nptl/pthread_create.c:311

Indirect leak of 699528 byte(s) in 2 object(s) allocated from:
    #0 0x7febb22214ab in __interceptor_malloc (/home/default/storage/skia-repo/buildbot/skiabot-linux-xsan-001/build/slave/Test-Ubuntu13_10-GCE-NoGPU-x86_64-Debug-ASAN/build/skia/out/Debug/dm+0x33a4ab)
    #1 0x7febb473e3c6 in sk_malloc_flags(unsigned long, unsigned int) /home/default/storage/skia-repo/buildbot/skiabot-linux-xsan-001/build/slave/Test-Ubuntu13_10-GCE-NoGPU-x86_64-Debug-ASAN/build/skia/out/Debug/../../src/ports/SkMemory_malloc.cpp:54:15
    #2 0x7febb473e19a in sk_malloc_throw(unsigned long) /home/default/storage/skia-repo/buildbot/skiabot-linux-xsan-001/build/slave/Test-Ubuntu13_10-GCE-NoGPU-x86_64-Debug-ASAN/build/skia/out/Debug/../../src/ports/SkMemory_malloc.cpp:40:12
    #3 0x7febb3483c94 in SkMipMap::Build(SkBitmap const&, SkDiscardableMemory* (*)(unsigned long)) /home/default/storage/skia-repo/buildbot/skiabot-linux-xsan-001/build/slave/Test-Ubuntu13_10-GCE-NoGPU-x86_64-Debug-ASAN/build/skia/out/Debug/../../src/core/SkMipMap.cpp:181:18
    #4 0x7febb2c37914 in ShowMipLevels::onDraw(SkCanvas*) /home/default/storage/skia-repo/buildbot/skiabot-linux-xsan-001/build/slave/Test-Ubuntu13_10-GCE-NoGPU-x86_64-Debug-ASAN/build/skia/out/Debug/../../gm/downsamplebitmap.cpp:228:24
    #5 0x7febb2295736 in skiagm::GM::drawContent(SkCanvas*) /home/default/storage/skia-repo/buildbot/skiabot-linux-xsan-001/build/slave/Test-Ubuntu13_10-GCE-NoGPU-x86_64-Debug-ASAN/build/skia/out/Debug/../../gm/gm.cpp:32:5
    #6 0x7febb2295019 in skiagm::GM::draw(SkCanvas*) /home/default/storage/skia-repo/buildbot/skiabot-linux-xsan-001/build/slave/Test-Ubuntu13_10-GCE-NoGPU-x86_64-Debug-ASAN/build/skia/out/Debug/../../gm/gm.cpp:24:5
    #7 0x7febb226722f in DM::GMSrc::draw(SkCanvas*) const /home/default/storage/skia-repo/buildbot/skiabot-linux-xsan-001/build/slave/Test-Ubuntu13_10-GCE-NoGPU-x86_64-Debug-ASAN/build/skia/out/Debug/../../dm/DMSrcSink.cpp:22:5
    #8 0x7febb226d929 in DM::RasterSink::draw(DM::Src const&, SkBitmap*, SkWStream*) const /home/default/storage/skia-repo/buildbot/skiabot-linux-xsan-001/build/slave/Test-Ubuntu13_10-GCE-NoGPU-x86_64-Debug-ASAN/build/skia/out/Debug/../../dm/DMSrcSink.cpp:230:12
    #9 0x7febb224e004 in Task::Run(Task*) /home/default/storage/skia-repo/buildbot/skiabot-linux-xsan-001/build/slave/Test-Ubuntu13_10-GCE-NoGPU-x86_64-Debug-ASAN/build/skia/out/Debug/../../dm/DM.cpp:254:25
    #10 0x7febb3805e4c in (anonymous namespace)::ThreadPool::Loop(void*) /home/default/storage/skia-repo/buildbot/skiabot-linux-xsan-001/build/slave/Test-Ubuntu13_10-GCE-NoGPU-x86_64-Debug-ASAN/build/skia/out/Debug/../../src/core/SkTaskGroup.cpp:149:13
    #11 0x7febb460dc7c in thread_start(void*) /home/default/storage/skia-repo/buildbot/skiabot-linux-xsan-001/build/slave/Test-Ubuntu13_10-GCE-NoGPU-x86_64-Debug-ASAN/build/skia/out/Debug/../../src/utils/SkThreadUtils_pthread.cpp:66:9
    #12 0x7febb1251f6d in start_thread /build/buildd/eglibc-2.17/nptl/pthread_create.c:311

Original issue's description:
> add gm to show miplevels
>
> BUG=skia:
> TBR=
>
> NOTREECHECKS=true
>
> Committed: https://skia.googlesource.com/skia/+/a598f4b773f066a939e0216a116c179b90550727

TBR=reed@google.com
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=skia:

Review URL: https://codereview.chromium.org/854103004
suyambulingamrm pushed a commit that referenced this issue Sep 3, 2015
…review.chromium.org/936943002/)

Reason for revert:
Strange blur problems on nexus 5

Original issue's description:
> I'd really like to land this before the branch so speedy reviews are appreciated.
>
> BUG=skia:
>
> Committed: https://skia.googlesource.com/skia/+/586d5d640b19860dfbbd903a5188da1bbbe87336

TBR=jvanverth@google.com,senorblanco@google.com,bsalomon@google.com,senorblanco@chromium.org,joshualitt@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=skia:

Review URL: https://codereview.chromium.org/956083002
suyambulingamrm pushed a commit that referenced this issue Sep 3, 2015
…140001 of https://codereview.chromium.org/949263002/)

Reason for revert:
breaking things

Original issue's description:
> Improve tracking of bound FBOs in GrGLGpu.
>
> Committed: https://skia.googlesource.com/skia/+/d2ad8eb5801e2c8c0fa544a6a776bb46eedde2a0
>
> Committed: https://skia.googlesource.com/skia/+/b2af2d8b83ca4774c3b3bb1e49bc72605faa9589

TBR=egdaniel@google.com
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true

Review URL: https://codereview.chromium.org/963973002
suyambulingamrm pushed a commit that referenced this issue Sep 3, 2015
…https://codereview.chromium.org/963953002/)

Reason for revert:
breaking iOS build.

Original issue's description:
> XPS, DM: add SkDocument::CreateXPS
>
> -   SkDocument::CreateXPS() function added, returns NULL on non-Windows OS.
>
> -   DM: (Windows only) an XPSSink is added, fails on non-Windows OS
>
> -   DM: Common code for PDFSink::draw and XPSSink::draw are factored into
>     draw_skdocument static function.
>
> -   SkDocument_XPS (Windows only) implementation of SkDocument via
>     SkXPSDevice.
>
> -   SkDocument_XPS_None (non-Windows) returns NULL for
>     SkDocument::CreateXPS().
>
> -   gyp/xps.gyp refactored.
>
> -   SkXPSDevice::drawTextOnPath removed (see http://crrev.com/925343003 )
>
> -   SkXPSDevice::drawPath supports conics via SkAutoConicToQuads.
>
> NOPRESUBMIT=true
>
> Committed: https://skia.googlesource.com/skia/+/00d39bcbfc8394a9b48b86b04ab06ec19091fa43

TBR=reed@google.com,bungeman@google.com,mtklein@google.com
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true

Review URL: https://codereview.chromium.org/978443002
suyambulingamrm pushed a commit that referenced this issue Sep 3, 2015
…e storage. (patchset #8 id:140001 of https://codereview.chromium.org/986343002/)

Reason for revert:
Still seeing bad text rendering in GMs with Nexus 2 and Nexus 3.

Original issue's description:
> Adjust distance field glyph and font atlas sizes to maximize storage.
>
> Because of high DPI devices, we need more room in the glyph
> atlas for the larger glyphs. These settings will allow 4 of
> the distance field glyphs to fit in one Plot (increasing
> the storage from 32 large glyphs to 128), and still permit
> us to handle glyphs up to a 312 point size.
>
> BUG=chromium:458791
>
> Committed: https://skia.googlesource.com/skia/+/d2737ad7dd8f4ea94a74034027014fd3d78923cb
>
> Committed: https://skia.googlesource.com/skia/+/07e245ffe1ca7f781590fcff904c2922c2269781

TBR=joshualitt@google.com,bsalomon@google.com
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=chromium:458791

Review URL: https://codereview.chromium.org/995863002
suyambulingamrm pushed a commit that referenced this issue Sep 3, 2015
#8 id:260001 of https://codereview.chromium.org/1038863003/)

Reason for revert:
Trying out revert to see if it fixes Android bots.

Original issue's description:
> WIP: Added support for giflib, updated jpeg and png
>
> BUG=skia:3257
>
> Committed: https://skia.googlesource.com/skia/+/255dcd11992ebe74eb54202c48cf5394d33a8ce6

TBR=djsollen@google.com,scroggo@google.com,msarett@google.com
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=skia:3257

Review URL: https://codereview.chromium.org/1048713003
suyambulingamrm pushed a commit that referenced this issue Sep 3, 2015
…001 of https://codereview.chromium.org/1061783002/)

Reason for revert:
https://uberchromegw.corp.google.com/i/client.skia/builders/Test-Ubuntu-GCC-GCE-CPU-AVX2-x86-Debug/builds/149/steps/dm/logs/stdio

Original issue's description:
> Rearrange SkRecord with small N in mind
>
> This rearranges the record pointers and types so they can go in a single array, then preallocates some space for them and for the SkVarAlloc.
>
> picture_overhead_draw bench drops from ~1000ns to 500-600ns, with no effect on picture_overhead_nodraw.
>
> I don't see any significant effect on large picture recording times from our .skps.
>
> BUG=chromium:470553
>
> Committed: https://skia.googlesource.com/skia/+/e2dd9408cd711777afaa9410427fb0d761ab004a

TBR=reed@google.com,mtklein@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=chromium:470553

Review URL: https://codereview.chromium.org/1068383003
suyambulingamrm pushed a commit that referenced this issue Nov 13, 2015
…1 of https://codereview.chromium.org/1295523002/ )

Reason for revert:
breaks layout tests

Original issue's description:
> stop dropping AA when rect stays rect
>
> BUG=skia:
>
> Committed: https://skia.googlesource.com/skia/+/1bef9f59c566cc54c2259cc4d0171c115157cd1c

TBR=bsalomon@google.com,robertphillips@google.com,joshualitt@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=skia:

Review URL: https://codereview.chromium.org/1295813005
suyambulingamrm pushed a commit that referenced this issue Nov 13, 2015
#8 id:130001 of https://codereview.chromium.org/1296943002/ )

Reason for revert:
This causes a syntax error.

http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_compile_dbg_ng/builds/87819/steps/compile%20%28with%20patch%29/logs/stdio

Original issue's description:
> Implement canComputeFastBounds() for image filters.
>
> Image filters have never implemented this check, which means that
> filters which affect transparent black falsely claim they can compute
> their bounds.
>
> Implemented an affectsTransparentBlack() virtual for image
> filters, and a similar helper function for color filters.
>
> This will affect the following GMs: imagefiltersscaled
> (lighting, perlin noise now filter to clip),
> colorfilterimagefilter (new test case), imagefiltersclipped
> (perlin noise now filters to clip).
>
> Note: I de-inlined SkPaint::canComputeFastBounds() to avoid adding
> a dependency from SkPaint.h to SkImageFilter.h.h. Skia benches show
> no impact from this change, but will watch the perf bots carefully.
>
> BUG=4212
>
> Committed: https://skia.googlesource.com/skia/+/915881fe743f9a789037695f543bc6ea189cd0cb

TBR=reed@google.com,senorblanco@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=4212

Review URL: https://codereview.chromium.org/1300403003
suyambulingamrm pushed a commit that referenced this issue Nov 13, 2015
#1 id:1 of https://codereview.chromium.org/1300403003/ )

Reason for revert:
The Mac compile issue was fixed here: https://chromium.googlesource.com/chromium/src/+/fdd331a42ae0b9a6909a121020735161ab61c6e5

Original issue's description:
> Revert of Implement canComputeFastBounds() for image filters. (patchset #8 id:130001 of https://codereview.chromium.org/1296943002/ )
>
> Reason for revert:
> This causes a syntax error.
>
> http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_compile_dbg_ng/builds/87819/steps/compile%20%28with%20patch%29/logs/stdio
>
> Original issue's description:
> > Implement canComputeFastBounds() for image filters.
> >
> > Image filters have never implemented this check, which means that
> > filters which affect transparent black falsely claim they can compute
> > their bounds.
> >
> > Implemented an affectsTransparentBlack() virtual for image
> > filters, and a similar helper function for color filters.
> >
> > This will affect the following GMs: imagefiltersscaled
> > (lighting, perlin noise now filter to clip),
> > colorfilterimagefilter (new test case), imagefiltersclipped
> > (perlin noise now filters to clip).
> >
> > Note: I de-inlined SkPaint::canComputeFastBounds() to avoid adding
> > a dependency from SkPaint.h to SkImageFilter.h.h. Skia benches show
> > no impact from this change, but will watch the perf bots carefully.
> >
> > BUG=4212
> >
> > Committed: https://skia.googlesource.com/skia/+/915881fe743f9a789037695f543bc6ea189cd0cb
>
> TBR=reed@google.com,senorblanco@chromium.org
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=4212
>
> Committed: https://skia.googlesource.com/skia/+/12d8472d31ea5edb636d7d5214db253570115c40

TBR=reed@google.com,herb@google.com
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=4212

Review URL: https://codereview.chromium.org/1301823005
suyambulingamrm pushed a commit that referenced this issue Nov 13, 2015
…take skpaint\grcolor* (patchset #8 id:140001 of https://codereview.chromium.org/1316513002/ )

Reason for revert:
Primary suspect in failing DEPS rolls:
* https://codereview.chromium.org/1315753006
* https://codereview.chromium.org/1308323006
* https://codereview.chromium.org/1320903004

Primary suspect because the failing win bots did not fail in https://codereview.chromium.org/1315753005

Original issue's description:
> Change SkShader;asFragmentProcessor signature to no longer take skpaint\grcolor*
>
> Committed: https://skia.googlesource.com/skia/+/ecfdc251be71f3d634e76afdd6375bf55fc061aa

TBR=joshualitt@google.com,wangyix@google.com,robertphillips@google.com,bsalomon@google.com
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true

Review URL: https://codereview.chromium.org/1313573005
suyambulingamrm pushed a commit that referenced this issue Nov 13, 2015
…set #8 id:140001 of https://codereview.chromium.org/1370323002/ )

Reason for revert:
Landed PS8 temporarily to trigger the perf bots.

Original issue's description:
> Implement SkImageFilter::Cache with SkResourceCache.
>
> The single global cache now uses the global SkResourceCache,
> and any Create()ed cache uses a local SkResourceCache.
>
> No real public API changes (and only deletes).
>
> I don't see any pixel diffs on .skps or GMs.
> Don't see any significant perf difference on 8888 or gpu configs.
> DM peak memory usage did drop by about 113M, close to the 128M cache size.
>
> BUG=skia:3662
>
> Landing PS8 temporarily to trigger the perf bots.
> TBR=reed@google.com
>
> Committed: https://skia.googlesource.com/skia/+/75135d8ae1aa12e8e6bfce63291e5e876a77546f

TBR=reed@google.com,robertphillips@google.com
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=skia:3662

Review URL: https://codereview.chromium.org/1381523002
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants