Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(76)

Issue 1405583002: Fix text axis alignment calculation (Closed)

Created:
5 years, 2 months ago by herb_g
Modified:
5 years, 2 months ago
Reviewers:
reed1
CC:
reviews_skia.org
Base URL:
https://skia.googlesource.com/skia.git@master
Target Ref:
refs/heads/master
Project:
skia
Visibility:
Public.

Description

Fix text axis alignment calculation BUG=skia: Committed: https://skia.googlesource.com/skia/+/eb85b8321bc917169ba26c8fce76b64d2e3dfe81

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1 line, -1 line) Patch
M src/core/SkScalerContext.cpp View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 9 (2 generated)
herb_g
5 years, 2 months ago (2015-10-13 16:18:24 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1405583002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1405583002/1
5 years, 2 months ago (2015-10-13 16:18:33 UTC) #4
commit-bot: I haz the power
Note for Reviewers: The CQ is waiting for an approval. If you believe that the ...
5 years, 2 months ago (2015-10-13 16:18:34 UTC) #5
reed1
lgtm
5 years, 2 months ago (2015-10-13 18:46:42 UTC) #6
commit-bot: I haz the power
Committed patchset #1 (id:1) as https://skia.googlesource.com/skia/+/eb85b8321bc917169ba26c8fce76b64d2e3dfe81
5 years, 2 months ago (2015-10-13 18:47:58 UTC) #7
bsalomon
On 2015/10/13 18:47:58, commit-bot: I haz the power wrote: > Committed patchset #1 (id:1) as ...
5 years, 2 months ago (2015-10-14 13:12:31 UTC) #8
bsalomon
5 years, 2 months ago (2015-10-14 13:13:55 UTC) #9
Message was sent while issue was closed.
On 2015/10/14 13:12:31, bsalomon wrote:
> On 2015/10/13 18:47:58, commit-bot: I haz the power wrote:
> > Committed patchset #1 (id:1) as
> >
https://skia.googlesource.com/skia/+/eb85b8321bc917169ba26c8fce76b64d2e3dfe81
> 
> Are the uninit errors on the valgrind bot attributable to this change?
> 
>
http://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-GCE-CPU-AVX2...
> 
> 
> ex:
> ==32300== Use of uninitialised value of size 8
> ==32300==    at 0x697F64: SkGlyphCache::lookupByPackedGlyphID(unsigned int,
> SkGlyphCache::MetricsType) (SkTHash.h:179)
> ==32300==    by 0x698010: SkGlyphCache::getGlyphIDMetrics(unsigned short)
> (SkGlyphCache.cpp:145)
> ==32300==    by 0x6AAA70: sk_getMetrics_glyph_next(SkGlyphCache*, char
const**)
> (SkPaint.cpp:579)
> ==32300==    by 0x6ADF35: SkPaint::getTextWidths(void const*, unsigned long,
> float*, SkRect*) const (SkPaint.cpp:1150)
> ==32300==    by 0x6E7933:
> SkTextBlobBuilder::TightRunBounds(SkTextBlob::RunRecord const&)
> (SkTextBlob.cpp:383)
> ==32300==    by 0x6E7BE0:
> SkTextBlobBuilder::ConservativeRunBounds(SkTextBlob::RunRecord const&)
> (SkTextBlob.cpp:421)
> ==32300==    by 0x6E7DA2: SkTextBlobBuilder::updateDeferredBounds()
> (SkTextBlob.cpp:474)
> ==32300==    by 0x6E829D: SkTextBlobBuilder::build() (SkTextBlob.cpp:614)
> ==32300==    by 0x58538E: TextBlobTester::TestBounds(skiatest::Reporter*)
> (TextBlobTest.cpp:165)
> ==32300==    by 0x584D45: test_TextBlob_builder(skiatest::Reporter*,
> GrContextFactory*) (TextBlobTest.cpp:257)
> ==32300==    by 0x408B8C: run_test(skiatest::Test*) (DM.cpp:980)
> ==32300==    by 0x6E6E11: (anonymous namespace)::ThreadPool::Loop(void*)
> (SkTaskGroup.cpp:176)
> ==32300==    by 0x7FA3B6: thread_start(void*) (SkThreadUtils_pthread.cpp:66)
> ==32300==    by 0x5058181: start_thread (pthread_create.c:312)
> ==32300==    by 0x628400C: clone (clone.S:111)
> ==32300==  Uninitialised value was created by a heap allocation
> ==32300==    at 0x4C2AB80: malloc (in
> /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==32300==    by 0x4C2CF1F: realloc (in
> /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==32300==    by 0x805DBA: sk_realloc_throw(void*, unsigned long)
> (SkMemory_malloc.cpp:44)
> ==32300==    by 0x6E7E35: SkTextBlobBuilder::reserve(unsigned long)
> (SkTemplates.h:271)
> ==32300==    by 0x6E8106: SkTextBlobBuilder::allocInternal(SkPaint const&,
> SkTextBlob::GlyphPositioning, int, SkPoint, SkRect const*)
(SkTextBlob.cpp:559)
> ==32300==    by 0x6E828A: SkTextBlobBuilder::allocRunPos(SkPaint const&, int,
> SkRect const*) (SkTextBlob.cpp:606)
> ==32300==    by 0x585344: TextBlobTester::TestBounds(skiatest::Reporter*)
> (TextBlobTest.cpp:162)
> ==32300==    by 0x584D45: test_TextBlob_builder(skiatest::Reporter*,
> GrContextFactory*) (TextBlobTest.cpp:257)
> ==32300==    by 0x408B8C: run_test(skiatest::Test*) (DM.cpp:980)
> ==32300==    by 0x6E6E11: (anonymous namespace)::ThreadPool::Loop(void*)
> (SkTaskGroup.cpp:176)
> ==32300==    by 0x7FA3B6: thread_start(void*) (SkThreadUtils_pthread.cpp:66)
> ==32300==    by 0x5058181: start_thread (pthread_create.c:312)
> ==32300==    by 0x628400C: clone (clone.S:111)
> ==32300== 
> {
>    <insert_a_suppression_name_here>
>    Memcheck:Value8
>    fun:_ZN12SkGlyphCache21lookupByPackedGlyphIDEjNS_11MetricsTypeE
>    fun:_ZN12SkGlyphCache17getGlyphIDMetricsEt
>    fun:_ZL24sk_getMetrics_glyph_nextP12SkGlyphCachePPKc
>    fun:_ZNK7SkPaint13getTextWidthsEPKvmPfP6SkRect
>    fun:_ZN17SkTextBlobBuilder14TightRunBoundsERKN10SkTextBlob9RunRecordE
>   
fun:_ZN17SkTextBlobBuilder21ConservativeRunBoundsERKN10SkTextBlob9RunRecordE
>    fun:_ZN17SkTextBlobBuilder20updateDeferredBoundsEv
>    fun:_ZN17SkTextBlobBuilder5buildEv
>    fun:_ZN14TextBlobTester10TestBoundsEPN8skiatest8ReporterE
>    fun:_ZL21test_TextBlob_builderPN8skiatest8ReporterEP16GrContextFactory
>    fun:_ZL8run_testPN8skiatest4TestE
>    fun:_ZN12_GLOBAL__N_110ThreadPool4LoopEPv
>    fun:_ZL12thread_startPv
>    fun:start_thread
>    fun:clone
> }

https://codereview.chromium.org/1405463004 is perhaps a more likely candidate.

Powered by Google App Engine
This is Rietveld 408576698