From 26bea5d55735367152378bae14453e9666d1c625 Mon Sep 17 00:00:00 2001 From: Mike Klein Date: Wed, 5 Oct 2016 10:36:38 -0400 Subject: [PATCH] Make test lower-level, make const_cast more visible. I can only think there's something funky going on with the hidden const_cast inside SkRasterPipeline.cpp, or with the Sk4h -> Sk4f conversion. So make the const_cast visible and write the test directly in halfs instead of floats. CQ_INCLUDE_TRYBOTS=master.client.skia:Test-Mac-Clang-MacMini6.2-CPU-AVX-x86_64-Release-GN-Trybot BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=3003 Change-Id: I3a7e19ae165fce44f177b4968a63ec04e25c4b93 Reviewed-on: https://skia-review.googlesource.com/3003 Reviewed-by: Mike Klein Commit-Queue: Mike Klein --- src/core/SkRasterPipeline.cpp | 8 ++++---- src/core/SkRasterPipeline.h | 5 +++-- tests/SkRasterPipelineTest.cpp | 26 +++++++------------------- 3 files changed, 14 insertions(+), 25 deletions(-) diff --git a/src/core/SkRasterPipeline.cpp b/src/core/SkRasterPipeline.cpp index fd909a1205..72d5b7b963 100644 --- a/src/core/SkRasterPipeline.cpp +++ b/src/core/SkRasterPipeline.cpp @@ -12,7 +12,7 @@ SkRasterPipeline::SkRasterPipeline() {} void SkRasterPipeline::append(SkRasterPipeline::Fn body, SkRasterPipeline::Fn tail, - const void* ctx) { + void* ctx) { // Each stage holds its own context and the next function to call. // So the pipeline itself has to hold onto the first function that starts the pipeline. (fBody.empty() ? fBodyStart : fBody.back().fNext) = body; @@ -20,11 +20,11 @@ void SkRasterPipeline::append(SkRasterPipeline::Fn body, // Each last stage starts with its next function set to JustReturn as a safety net. // It'll be overwritten by the next call to append(). - fBody.push_back({ &JustReturn, const_cast(ctx) }); - fTail.push_back({ &JustReturn, const_cast(ctx) }); + fBody.push_back({ &JustReturn, ctx }); + fTail.push_back({ &JustReturn, ctx }); } -void SkRasterPipeline::append(StockStage stage, const void* ctx) { +void SkRasterPipeline::append(StockStage stage, void* ctx) { this->append(SkOpts::stages_4[stage], SkOpts::stages_1_3[stage], ctx); } diff --git a/src/core/SkRasterPipeline.h b/src/core/SkRasterPipeline.h index 785a118ad8..996c7838e3 100644 --- a/src/core/SkRasterPipeline.h +++ b/src/core/SkRasterPipeline.h @@ -131,7 +131,8 @@ public: kNumStockStages, }; - void append(StockStage, const void* ctx=nullptr); + void append(StockStage, void* = nullptr); + void append(StockStage stage, const void* ctx) { this->append(stage, const_cast(ctx)); } // Append all stages to this pipeline. @@ -140,7 +141,7 @@ public: private: using Stages = SkSTArray<10, Stage, /*MEM_COPY=*/true>; - void append(Fn body, Fn tail, const void*); + void append(Fn body, Fn tail, void*); // This no-op default makes fBodyStart and fTailStart unconditionally safe to call, // and is always the last stage's fNext as a sort of safety net to make sure even a diff --git a/tests/SkRasterPipelineTest.cpp b/tests/SkRasterPipelineTest.cpp index 2014671933..76fbc2541c 100644 --- a/tests/SkRasterPipelineTest.cpp +++ b/tests/SkRasterPipelineTest.cpp @@ -12,16 +12,9 @@ DEF_TEST(SkRasterPipeline, r) { // Build and run a simple pipeline to exercise SkRasterPipeline, // drawing 50% transparent blue over opaque red in half-floats. - - Sk4h red = SkFloatToHalf_finite_ftz({ 1.0f, 0.0f, 0.0f, 1.0f }), - blue = SkFloatToHalf_finite_ftz({ 0.0f, 0.0f, 0.5f, 0.5f }), - result = { 1, 2, 3, 4 }; - - uint64_t bits; - memcpy(&bits, &red, 8); - SkDebugf("SkRasterPipeline red: 0x%016llx, want 0x3c00000000003c00\n", bits); - memcpy(&bits, &blue, 8); - SkDebugf("SkRasterPipeline blue: 0x%016llx, want 0x3800380000000000\n", bits); + uint64_t red = 0x3c00000000003c00ull, + blue = 0x3800380000000000ull, + result; SkRasterPipeline p; p.append(SkRasterPipeline::load_s_f16, &blue); @@ -30,16 +23,11 @@ DEF_TEST(SkRasterPipeline, r) { p.append(SkRasterPipeline::store_f16, &result); p.run(1); - Sk4f f = SkHalfToFloat_finite_ftz(result); - - memcpy(&bits, &result, 8); - SkDebugf("SkRasterPipeline result: 0x%016llx, want 0x3c00380000003800\n", bits); - // We should see half-intensity magenta. - REPORTER_ASSERT(r, f[0] == 0.5f); - REPORTER_ASSERT(r, f[1] == 0.0f); - REPORTER_ASSERT(r, f[2] == 0.5f); - REPORTER_ASSERT(r, f[3] == 1.0f); + REPORTER_ASSERT(r, ((result >> 0) & 0xffff) == 0x3800); + REPORTER_ASSERT(r, ((result >> 16) & 0xffff) == 0x0000); + REPORTER_ASSERT(r, ((result >> 32) & 0xffff) == 0x3800); + REPORTER_ASSERT(r, ((result >> 48) & 0xffff) == 0x3c00); } DEF_TEST(SkRasterPipeline_empty, r) {