Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1870463002:
flesh out gradient shaders (Closed)
Descriptionflesh out gradient shaders
Using these webpages as guides
http://www.globelegislators.org/pdfjs/test/pdfs/alphatrans.pdf
http://www.antennahouse.com/antenna1/wp-content/uploads/2015/07/background-image-gradient-1.pdf
flesh out the gradient shaders to include the PostScript Type 0
sampling function and to include radial gradients. This CL makes
rendering these pages agree with Adobe Reader output.
Some of these examples use an Extend array to clip the gradient.
Skia does not currently support this natively, so construct the
clip manually for now.
Other PDF pages may construct gradients using alternate mechanisms
-- this code will continue to be refactored as those come to light.
In particular, this CL sets up the clip and matrix differently for
axial gradients and radial gradients -- while it makes sense to do
it one way only, I prefer to defer until I have more examples to
work with.
R=dsinclair@chromium.org,tsepez@chromium.org
Committed: https://pdfium.googlesource.com/pdfium/+/f9924422a4cd1d1b6d2d4240c83b5c1082da3629
Patch Set 1 #Patch Set 2 : wip; more work on linear gradients #Patch Set 3 : wip; add radial gradients #Patch Set 4 : wip; fix radial clipping #Patch Set 5 : add radial gradient support #Patch Set 6 : add comment #
Total comments: 6
Patch Set 7 : rebase #Patch Set 8 : address comments #Patch Set 9 : address comments #
Total comments: 36
Patch Set 10 : add unit test #Patch Set 11 : fix unittest gyp #Patch Set 12 : fix linux-detected error #
Total comments: 1
Messages
Total messages: 42 (19 generated)
Description was changed from ========== wip; add sampled function support some gradients use type 0 sampled functions; support them BUG= ========== to ========== flesh out gradient shaders Using these webpages as guides http://www.globelegislators.org/pdfjs/test/pdfs/alphatrans.pdf https://www.antenna.co.jp/AHF/ahf_samples/ja/fo/graphics/background-image-gra... flesh out the gradient shaders to include the PostScript Type 0 sampling function and to include radial gradients. This CL makes rendering these pages agree with Adobe Reader output. Some of these examples use an Extend array to clip the gradient. Skia does not currently support this natively, so construct the clip manually for now. Other PDF pages may construct gradients using alternate mechanisms -- this code will continue to be refactored as those come to light. In particular, this CL sets up the clip and matrix differently for axial gradients and radial gradients -- while it makes sense to do it one way only, I prefer to defer until I have more examples to work with. R=dsinclair@chromium.org,tsepez@chromium.org ==========
caryclark@google.com changed reviewers: + dsinclair@chromium.org, tsepez@chromium.org
The CQ bit was checked by caryclark@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1870463002/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1870463002/100001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_xfa on master.tryserver.client.pdfium (JOB_FAILED, https://build.chromium.org/p/tryserver.client.pdfium/builders/linux_xfa/build...)
The CQ bit was checked by caryclark@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1870463002/120001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1870463002/120001
https://codereview.chromium.org/1870463002/diff/100001/core/fpdfapi/fpdf_page... File core/fpdfapi/fpdf_page/fpdf_page_func.cpp (right): https://codereview.chromium.org/1870463002/diff/100001/core/fpdfapi/fpdf_page... core/fpdfapi/fpdf_page/fpdf_page_func.cpp:638: class CPDF_PSFunc : public CPDF_Function { Can this be moved up into the namespace {} at the top? https://codereview.chromium.org/1870463002/diff/100001/core/fxge/skia/fx_skia... File core/fxge/skia/fx_skia_device.cpp (right): https://codereview.chromium.org/1870463002/diff/100001/core/fxge/skia/fx_skia... core/fxge/skia/fx_skia_device.cpp:184: int bitShift, bitMask, dstShift; nit: one per line. https://codereview.chromium.org/1870463002/diff/100001/core/fxge/skia/fx_skia... core/fxge/skia/fx_skia_device.cpp:883: // skMatrix.mapPoints(pts, SK_ARRAY_COUNT(pts)); nit: remove
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by caryclark@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1870463002/160001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1870463002/160001
https://codereview.chromium.org/1870463002/diff/100001/core/fpdfapi/fpdf_page... File core/fpdfapi/fpdf_page/fpdf_page_func.cpp (right): https://codereview.chromium.org/1870463002/diff/100001/core/fpdfapi/fpdf_page... core/fpdfapi/fpdf_page/fpdf_page_func.cpp:638: class CPDF_PSFunc : public CPDF_Function { On 2016/04/11 14:29:28, dsinclair wrote: > Can this be moved up into the namespace {} at the top? Done. https://codereview.chromium.org/1870463002/diff/100001/core/fxge/skia/fx_skia... File core/fxge/skia/fx_skia_device.cpp (right): https://codereview.chromium.org/1870463002/diff/100001/core/fxge/skia/fx_skia... core/fxge/skia/fx_skia_device.cpp:184: int bitShift, bitMask, dstShift; On 2016/04/11 14:29:28, dsinclair wrote: > nit: one per line. Done. https://codereview.chromium.org/1870463002/diff/100001/core/fxge/skia/fx_skia... core/fxge/skia/fx_skia_device.cpp:883: // skMatrix.mapPoints(pts, SK_ARRAY_COUNT(pts)); On 2016/04/11 14:29:28, dsinclair wrote: > nit: remove Done.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/1870463002/diff/160001/core/fpdfapi/fpdf_page... File core/fpdfapi/fpdf_page/fpdf_page_func.cpp (right): https://codereview.chromium.org/1870463002/diff/160001/core/fpdfapi/fpdf_page... core/fpdfapi/fpdf_page/fpdf_page_func.cpp:524: } // namespace Hm, did this make the namespace section smaller? It looks like it used to end of ~682? Hard to tell from the diff.
https://codereview.chromium.org/1870463002/diff/160001/core/fpdfapi/fpdf_page... File core/fpdfapi/fpdf_page/fpdf_page_func.cpp (right): https://codereview.chromium.org/1870463002/diff/160001/core/fpdfapi/fpdf_page... core/fpdfapi/fpdf_page/fpdf_page_func.cpp:524: } // namespace On 2016/04/11 16:17:45, dsinclair wrote: > Hm, did this make the namespace section smaller? It looks like it used to end of > ~682? Hard to tell from the diff. There were two namespace blocks. Now there is one. diff --git a/core/fpdfapi/fpdf_page/fpdf_page_func.cpp b/core/fpdfapi/fpdf_page/fpdf_page_func.cpp index e3651a1..3e71d90 100644 --- a/core/fpdfapi/fpdf_page/fpdf_page_func.cpp +++ b/core/fpdfapi/fpdf_page/fpdf_page_func.cpp @@ -492,6 +492,35 @@ static uint32_t _GetBits32(const uint8_t* pData, int bitpos, int nbits) { return result; } +class CPDF_PSFunc : public CPDF_Function { + public: + // CPDF_Function + CPDF_PSFunc() : CPDF_Function(Type::kType4PostScript) {} + FX_BOOL v_Init(CPDF_Object* pObj) override; + FX_BOOL v_Call(FX_FLOAT* inputs, FX_FLOAT* results) const override; + + CPDF_PSEngine m_PS; +}; + +FX_BOOL CPDF_PSFunc::v_Init(CPDF_Object* pObj) { + CPDF_Stream* pStream = pObj->AsStream(); + CPDF_StreamAcc acc; + acc.LoadAllData(pStream, FALSE); + return m_PS.Parse((const FX_CHAR*)acc.GetData(), acc.GetSize()); +} +FX_BOOL CPDF_PSFunc::v_Call(FX_FLOAT* inputs, FX_FLOAT* results) const { + CPDF_PSEngine& PS = (CPDF_PSEngine&)m_PS; + PS.Reset(); + for (uint32_t i = 0; i < m_nInputs; i++) + PS.Push(inputs[i]); + PS.Execute(); + if (PS.GetStackSize() < m_nOutputs) + return FALSE; + for (uint32_t i = 0; i < m_nOutputs; i++) + results[m_nOutputs - i - 1] = PS.Pop(); + return TRUE; +} + } // namespace CPDF_SampledFunc::CPDF_SampledFunc() : CPDF_Function(Type::kType0Sampled) { @@ -633,39 +662,6 @@ FX_BOOL CPDF_SampledFunc::v_Call(FX_FLOAT* inputs, FX_FLOAT* results) const { return TRUE; } -namespace { - -class CPDF_PSFunc : public CPDF_Function { - public: - // CPDF_Function - CPDF_PSFunc() : CPDF_Function(Type::kType4PostScript) {} - FX_BOOL v_Init(CPDF_Object* pObj) override; - FX_BOOL v_Call(FX_FLOAT* inputs, FX_FLOAT* results) const override; - - CPDF_PSEngine m_PS; -}; - -FX_BOOL CPDF_PSFunc::v_Init(CPDF_Object* pObj) { - CPDF_Stream* pStream = pObj->AsStream(); - CPDF_StreamAcc acc; - acc.LoadAllData(pStream, FALSE); - return m_PS.Parse((const FX_CHAR*)acc.GetData(), acc.GetSize()); -} -FX_BOOL CPDF_PSFunc::v_Call(FX_FLOAT* inputs, FX_FLOAT* results) const { - CPDF_PSEngine& PS = (CPDF_PSEngine&)m_PS; - PS.Reset(); - for (uint32_t i = 0; i < m_nInputs; i++) - PS.Push(inputs[i]); - PS.Execute(); - if (PS.GetStackSize() < m_nOutputs) - return FALSE; - for (uint32_t i = 0; i < m_nOutputs; i++) - results[m_nOutputs - i - 1] = PS.Pop(); - return TRUE; -} - -} // namespace - CPDF_ExpIntFunc::CPDF_ExpIntFunc() : CPDF_Function(Type::kType2ExpotentialInterpolation) {
https://codereview.chromium.org/1870463002/diff/160001/core/fpdfapi/fpdf_page... File core/fpdfapi/fpdf_page/fpdf_page_func.cpp (right): https://codereview.chromium.org/1870463002/diff/160001/core/fpdfapi/fpdf_page... core/fpdfapi/fpdf_page/fpdf_page_func.cpp:498: CPDF_PSFunc() : CPDF_Function(Type::kType4PostScript) {} nit: ctor should come before this comment, which indicates that the following overrides override methods in ... https://codereview.chromium.org/1870463002/diff/160001/core/fpdfapi/fpdf_page... core/fpdfapi/fpdf_page/fpdf_page_func.cpp:502: CPDF_PSEngine m_PS; nit: can this be private or protected? https://codereview.chromium.org/1870463002/diff/160001/core/fpdfapi/fpdf_page... core/fpdfapi/fpdf_page/fpdf_page_func.cpp:506: CPDF_Stream* pStream = pObj->AsStream(); nit: local not needed since it is only used once. https://codereview.chromium.org/1870463002/diff/160001/core/fpdfapi/fpdf_page... core/fpdfapi/fpdf_page/fpdf_page_func.cpp:509: return m_PS.Parse((const FX_CHAR*)acc.GetData(), acc.GetSize()); nit: reinterpret_cast<const FX_CHAR*> https://codereview.chromium.org/1870463002/diff/160001/core/fpdfapi/fpdf_page... core/fpdfapi/fpdf_page/fpdf_page_func.cpp:511: FX_BOOL CPDF_PSFunc::v_Call(FX_FLOAT* inputs, FX_FLOAT* results) const { nit: blank line here. https://codereview.chromium.org/1870463002/diff/160001/core/fpdfapi/fpdf_page... core/fpdfapi/fpdf_page/fpdf_page_func.cpp:512: CPDF_PSEngine& PS = (CPDF_PSEngine&)m_PS; nit: this isn't buying us anything over just using m_PS itself in the subsequent lines ... since the cast isn't doing anything? https://codereview.chromium.org/1870463002/diff/160001/core/fpdfapi/fpdf_page... File core/fpdfapi/fpdf_page/pageint.h (right): https://codereview.chromium.org/1870463002/diff/160001/core/fpdfapi/fpdf_page... core/fpdfapi/fpdf_page/pageint.h:432: int sizes; should this be size_t or at least unsigned? https://codereview.chromium.org/1870463002/diff/160001/core/fxge/skia/fx_skia... File core/fxge/skia/fx_skia_device.cpp (right): https://codereview.chromium.org/1870463002/diff/160001/core/fxge/skia/fx_skia... core/fxge/skia/fx_skia_device.cpp:181: uint32_t GetBits32(const uint8_t* pData, int bitpos, int nbits) { Presumably, this is copied from some other code that provides a unit test for this? https://codereview.chromium.org/1870463002/diff/160001/core/fxge/skia/fx_skia... core/fxge/skia/fx_skia_device.cpp:190: dstShift = 0; what if say, bitpos == 7 and nbits == 7? then 1 << -6 occurs, which is undefined behaviour. https://codereview.chromium.org/1870463002/diff/160001/core/fxge/skia/fx_skia... core/fxge/skia/fx_skia_device.cpp:194: dstShift = nbits - 8 + (bitpos & 0x07); what if, say, bitpos = 7 and nbits == 32, then we get dstShift = 31? https://codereview.chromium.org/1870463002/diff/160001/core/fxge/skia/fx_skia... core/fxge/skia/fx_skia_device.cpp:196: uint32_t result = (*dataPtr++ >> bitShift & bitMask) << dstShift; Then we get dataptr's contents as a uint8_t, promote it to int, shift and mask, and get an int in the 0 - 255. we could then shift an int by 31 bits, shifting into the signed bit, which is undefined behaviour. There needs to be a cast to uint32_t earlier on to avoid the default promotion to int rules. https://codereview.chromium.org/1870463002/diff/160001/core/fxge/skia/fx_skia... core/fxge/skia/fx_skia_device.cpp:232: int sampleSize = sampledFunc->m_nBitsPerSample; should be uint32_t to match m_nBitsPerSample? https://codereview.chromium.org/1870463002/diff/160001/core/fxge/skia/fx_skia... core/fxge/skia/fx_skia_device.cpp:243: for (int i = 0; i < sampleCount; ++i) { how do we know that sampleCount is within the bounds of m_pSampleStream's GetData() result? https://codereview.chromium.org/1870463002/diff/160001/core/fxge/skia/fx_skia... core/fxge/skia/fx_skia_device.cpp:386: // see en.wikipedia.org/wiki/Distance_from_a_point_to_a_line nit: if you put https:// in front of this, the link may be clickable when the source viewed in codesearch https://codereview.chromium.org/1870463002/diff/160001/core/fxge/skia/fx_skia... core/fxge/skia/fx_skia_device.cpp:387: SkScalar LineSide(const SkPoint l[2], const SkPoint& pt) { nit: any chance we could call "l" something like "line" so it doesn't look like "1" to my old eyes? https://codereview.chromium.org/1870463002/diff/160001/core/fxge/skia/fx_skia... core/fxge/skia/fx_skia_device.cpp:399: numerA /= denom; do we need to check for div by 0? https://codereview.chromium.org/1870463002/diff/160001/core/fxge/skia/fx_skia... core/fxge/skia/fx_skia_device.cpp:782: pPattern->m_ShadingType) { // TODO(caryclark) more types nit: this might look better if there's a newline before the //
https://codereview.chromium.org/1870463002/diff/160001/core/fpdfapi/fpdf_page... File core/fpdfapi/fpdf_page/fpdf_page_func.cpp (right): https://codereview.chromium.org/1870463002/diff/160001/core/fpdfapi/fpdf_page... core/fpdfapi/fpdf_page/fpdf_page_func.cpp:498: CPDF_PSFunc() : CPDF_Function(Type::kType4PostScript) {} On 2016/04/11 16:41:49, Tom Sepez wrote: > nit: ctor should come before this comment, which indicates that the following > overrides override methods in ... Done. https://codereview.chromium.org/1870463002/diff/160001/core/fpdfapi/fpdf_page... core/fpdfapi/fpdf_page/fpdf_page_func.cpp:502: CPDF_PSEngine m_PS; On 2016/04/11 16:41:49, Tom Sepez wrote: > nit: can this be private or protected? Done. https://codereview.chromium.org/1870463002/diff/160001/core/fpdfapi/fpdf_page... core/fpdfapi/fpdf_page/fpdf_page_func.cpp:506: CPDF_Stream* pStream = pObj->AsStream(); On 2016/04/11 16:41:49, Tom Sepez wrote: > nit: local not needed since it is only used once. Done. https://codereview.chromium.org/1870463002/diff/160001/core/fpdfapi/fpdf_page... core/fpdfapi/fpdf_page/fpdf_page_func.cpp:509: return m_PS.Parse((const FX_CHAR*)acc.GetData(), acc.GetSize()); On 2016/04/11 16:41:49, Tom Sepez wrote: > nit: reinterpret_cast<const FX_CHAR*> Done. https://codereview.chromium.org/1870463002/diff/160001/core/fpdfapi/fpdf_page... core/fpdfapi/fpdf_page/fpdf_page_func.cpp:511: FX_BOOL CPDF_PSFunc::v_Call(FX_FLOAT* inputs, FX_FLOAT* results) const { On 2016/04/11 16:41:49, Tom Sepez wrote: > nit: blank line here. Done. https://codereview.chromium.org/1870463002/diff/160001/core/fpdfapi/fpdf_page... core/fpdfapi/fpdf_page/fpdf_page_func.cpp:512: CPDF_PSEngine& PS = (CPDF_PSEngine&)m_PS; On 2016/04/11 16:41:49, Tom Sepez wrote: > nit: this isn't buying us anything over just using m_PS itself in the subsequent > lines ... since the cast isn't doing anything? v_Call is a const function; PS.Reset() for instance is a non-const member. I put in an explicit const cast to make it more clear. https://codereview.chromium.org/1870463002/diff/160001/core/fpdfapi/fpdf_page... File core/fpdfapi/fpdf_page/pageint.h (right): https://codereview.chromium.org/1870463002/diff/160001/core/fpdfapi/fpdf_page... core/fpdfapi/fpdf_page/pageint.h:432: int sizes; On 2016/04/11 16:41:49, Tom Sepez wrote: > should this be size_t or at least unsigned? Done. https://codereview.chromium.org/1870463002/diff/160001/core/fxge/skia/fx_skia... File core/fxge/skia/fx_skia_device.cpp (right): https://codereview.chromium.org/1870463002/diff/160001/core/fxge/skia/fx_skia... core/fxge/skia/fx_skia_device.cpp:181: uint32_t GetBits32(const uint8_t* pData, int bitpos, int nbits) { On 2016/04/11 16:41:50, Tom Sepez wrote: > Presumably, this is copied from some other code that provides a unit test for > this? Unit test added. https://codereview.chromium.org/1870463002/diff/160001/core/fxge/skia/fx_skia... core/fxge/skia/fx_skia_device.cpp:190: dstShift = 0; On 2016/04/11 16:41:50, Tom Sepez wrote: > what if say, bitpos == 7 and nbits == 7? then 1 << -6 occurs, which is undefined > behaviour. Fixed. https://codereview.chromium.org/1870463002/diff/160001/core/fxge/skia/fx_skia... core/fxge/skia/fx_skia_device.cpp:194: dstShift = nbits - 8 + (bitpos & 0x07); On 2016/04/11 16:41:50, Tom Sepez wrote: > what if, say, bitpos = 7 and nbits == 32, then we get > dstShift = 31? Fixed. https://codereview.chromium.org/1870463002/diff/160001/core/fxge/skia/fx_skia... core/fxge/skia/fx_skia_device.cpp:196: uint32_t result = (*dataPtr++ >> bitShift & bitMask) << dstShift; On 2016/04/11 16:41:50, Tom Sepez wrote: > Then we get dataptr's contents as a uint8_t, promote it to int, shift and mask, > and get an int in the 0 - 255. we could then shift an int by 31 bits, shifting > into the signed bit, which is undefined behaviour. There needs to be a cast to > uint32_t earlier on to avoid the default promotion to int rules. Fixed. https://codereview.chromium.org/1870463002/diff/160001/core/fxge/skia/fx_skia... core/fxge/skia/fx_skia_device.cpp:232: int sampleSize = sampledFunc->m_nBitsPerSample; On 2016/04/11 16:41:49, Tom Sepez wrote: > should be uint32_t to match m_nBitsPerSample? Done. https://codereview.chromium.org/1870463002/diff/160001/core/fxge/skia/fx_skia... core/fxge/skia/fx_skia_device.cpp:243: for (int i = 0; i < sampleCount; ++i) { On 2016/04/11 16:41:50, Tom Sepez wrote: > how do we know that sampleCount is within the bounds of m_pSampleStream's > GetData() result? Added check. https://codereview.chromium.org/1870463002/diff/160001/core/fxge/skia/fx_skia... core/fxge/skia/fx_skia_device.cpp:386: // see en.wikipedia.org/wiki/Distance_from_a_point_to_a_line On 2016/04/11 16:41:50, Tom Sepez wrote: > nit: if you put https:// in front of this, the link may be clickable when the > source viewed in codesearch Done. https://codereview.chromium.org/1870463002/diff/160001/core/fxge/skia/fx_skia... core/fxge/skia/fx_skia_device.cpp:387: SkScalar LineSide(const SkPoint l[2], const SkPoint& pt) { On 2016/04/11 16:41:50, Tom Sepez wrote: > nit: any chance we could call "l" something like "line" so it doesn't look like > "1" to my old eyes? Done. https://codereview.chromium.org/1870463002/diff/160001/core/fxge/skia/fx_skia... core/fxge/skia/fx_skia_device.cpp:399: numerA /= denom; On 2016/04/11 16:41:49, Tom Sepez wrote: > do we need to check for div by 0? Done. https://codereview.chromium.org/1870463002/diff/160001/core/fxge/skia/fx_skia... core/fxge/skia/fx_skia_device.cpp:782: pPattern->m_ShadingType) { // TODO(caryclark) more types On 2016/04/11 16:41:50, Tom Sepez wrote: > nit: this might look better if there's a newline before the // Done.
The CQ bit was checked by caryclark@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1870463002/180001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1870463002/180001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: mac_no_v8 on master.tryserver.client.pdfium (JOB_FAILED, https://build.chromium.org/p/tryserver.client.pdfium/builders/mac_no_v8/build...) win on master.tryserver.client.pdfium (JOB_FAILED, https://build.chromium.org/p/tryserver.client.pdfium/builders/win/builds/179)
The CQ bit was checked by caryclark@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1870463002/200001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1870463002/200001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux on master.tryserver.client.pdfium (JOB_FAILED, https://build.chromium.org/p/tryserver.client.pdfium/builders/linux/builds/181)
LGTM once the compilation on linux is fixed. Thanks.
The CQ bit was checked by caryclark@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1870463002/220001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1870463002/220001
Description was changed from ========== flesh out gradient shaders Using these webpages as guides http://www.globelegislators.org/pdfjs/test/pdfs/alphatrans.pdf https://www.antenna.co.jp/AHF/ahf_samples/ja/fo/graphics/background-image-gra... flesh out the gradient shaders to include the PostScript Type 0 sampling function and to include radial gradients. This CL makes rendering these pages agree with Adobe Reader output. Some of these examples use an Extend array to clip the gradient. Skia does not currently support this natively, so construct the clip manually for now. Other PDF pages may construct gradients using alternate mechanisms -- this code will continue to be refactored as those come to light. In particular, this CL sets up the clip and matrix differently for axial gradients and radial gradients -- while it makes sense to do it one way only, I prefer to defer until I have more examples to work with. R=dsinclair@chromium.org,tsepez@chromium.org ========== to ========== flesh out gradient shaders Using these webpages as guides http://www.globelegislators.org/pdfjs/test/pdfs/alphatrans.pdf http://www.antennahouse.com/antenna1/wp-content/uploads/2015/07/background-im... flesh out the gradient shaders to include the PostScript Type 0 sampling function and to include radial gradients. This CL makes rendering these pages agree with Adobe Reader output. Some of these examples use an Extend array to clip the gradient. Skia does not currently support this natively, so construct the clip manually for now. Other PDF pages may construct gradients using alternate mechanisms -- this code will continue to be refactored as those come to light. In particular, this CL sets up the clip and matrix differently for axial gradients and radial gradients -- while it makes sense to do it one way only, I prefer to defer until I have more examples to work with. R=dsinclair@chromium.org,tsepez@chromium.org ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by caryclark@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from tsepez@chromium.org Link to the patchset: https://codereview.chromium.org/1870463002/#ps220001 (title: "fix linux-detected error")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1870463002/220001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1870463002/220001
Message was sent while issue was closed.
Description was changed from ========== flesh out gradient shaders Using these webpages as guides http://www.globelegislators.org/pdfjs/test/pdfs/alphatrans.pdf http://www.antennahouse.com/antenna1/wp-content/uploads/2015/07/background-im... flesh out the gradient shaders to include the PostScript Type 0 sampling function and to include radial gradients. This CL makes rendering these pages agree with Adobe Reader output. Some of these examples use an Extend array to clip the gradient. Skia does not currently support this natively, so construct the clip manually for now. Other PDF pages may construct gradients using alternate mechanisms -- this code will continue to be refactored as those come to light. In particular, this CL sets up the clip and matrix differently for axial gradients and radial gradients -- while it makes sense to do it one way only, I prefer to defer until I have more examples to work with. R=dsinclair@chromium.org,tsepez@chromium.org ========== to ========== flesh out gradient shaders Using these webpages as guides http://www.globelegislators.org/pdfjs/test/pdfs/alphatrans.pdf http://www.antennahouse.com/antenna1/wp-content/uploads/2015/07/background-im... flesh out the gradient shaders to include the PostScript Type 0 sampling function and to include radial gradients. This CL makes rendering these pages agree with Adobe Reader output. Some of these examples use an Extend array to clip the gradient. Skia does not currently support this natively, so construct the clip manually for now. Other PDF pages may construct gradients using alternate mechanisms -- this code will continue to be refactored as those come to light. In particular, this CL sets up the clip and matrix differently for axial gradients and radial gradients -- while it makes sense to do it one way only, I prefer to defer until I have more examples to work with. R=dsinclair@chromium.org,tsepez@chromium.org Committed: https://pdfium.googlesource.com/pdfium/+/f9924422a4cd1d1b6d2d4240c83b5c1082da... ==========
Message was sent while issue was closed.
Committed patchset #12 (id:220001) as https://pdfium.googlesource.com/pdfium/+/f9924422a4cd1d1b6d2d4240c83b5c1082da...
Message was sent while issue was closed.
thestig@chromium.org changed reviewers: + thestig@chromium.org
Message was sent while issue was closed.
https://codereview.chromium.org/1870463002/diff/220001/pdfium.gyp File pdfium.gyp (right): https://codereview.chromium.org/1870463002/diff/220001/pdfium.gyp#newcode958 pdfium.gyp:958: ['pdf_use_skia==1', { BTW, this CL forgot to update BUILD.gn: + if (pdf_use_skia) { + sources += [ "core/fxge/skia/fx_skia_device_unittest.cpp" ] + deps += [ "//skia" ] + } |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
