Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2219683002:
Generate default AP stream for square annotation. (Closed)
DescriptionGenerate default AP stream for square annotation.
This patch generates a default AP stream for square annotation so that square
annotations without AP stream can be displayed.
Also, roll DEPS for testing/corpus to 7f07c22 to test square annotations.
BUG=62625
Committed: https://pdfium.googlesource.com/pdfium/+/af7ab33c73f58f18d0db0c90d93fa0aab0bc83f3
Patch Set 1 #
Total comments: 16
Patch Set 2 : Generate default AP stream for square annotation. #Patch Set 3 : Generate default AP stream for square annotation. #
Total comments: 10
Patch Set 4 : Generate default AP stream for square annotation. #
Total comments: 4
Patch Set 5 : Generate default AP stream for square annotation. #Patch Set 6 : Generate default AP stream for square annotation. #
Total comments: 2
Patch Set 7 : Generate default AP stream for square annotation. #
Total comments: 4
Patch Set 8 : Generate default AP stream for square annotation. #Patch Set 9 : Generate default AP stream for square annotation. #
Messages
Total messages: 46 (26 generated)
jaepark@google.com changed reviewers: + thestig@chromium.org
Although the test cases (https://codereview.chromium.org/2219683002) are not finalized yet, I hope to get an initial review.
https://codereview.chromium.org/2219683002/diff/1/core/fpdfdoc/cpvt_generatea... File core/fpdfdoc/cpvt_generateap.cpp (right): https://codereview.chromium.org/2219683002/diff/1/core/fpdfdoc/cpvt_generatea... core/fpdfdoc/cpvt_generateap.cpp:455: CFX_ByteString GetColorStringWithDefault(CPDF_Array* pColor, What prompted this change? Seemed fine before. https://codereview.chromium.org/2219683002/diff/1/core/fpdfdoc/cpvt_generatea... core/fpdfdoc/cpvt_generateap.cpp:469: return pBorderStyleDict->GetNumberBy("W"); Do you care if this or line 473 returns 0? If a malformed PDF returns a negative number, is that ok? https://codereview.chromium.org/2219683002/diff/1/core/fpdfdoc/cpvt_generatea... core/fpdfdoc/cpvt_generateap.cpp:478: CFX_ByteString GetDashArrayInString(const CPDF_Dictionary& pAnnotDict) { just GetDashArrayString? https://codereview.chromium.org/2219683002/diff/1/core/fpdfdoc/cpvt_generatea... core/fpdfdoc/cpvt_generateap.cpp:491: sDashStream << "[" << pDashArray->GetNumberAt(0); If |pDashArray| is empty, you would get "[ 0 ]", is that ok? https://codereview.chromium.org/2219683002/diff/1/core/fpdfdoc/cpvt_generatea... core/fpdfdoc/cpvt_generateap.cpp:668: bool isFillRect = pAnnotDict->GetArrayBy("IC") && bIsFillRect for consistency. https://codereview.chromium.org/2219683002/diff/1/core/fpdfdoc/cpvt_generatea... core/fpdfdoc/cpvt_generateap.cpp:668: bool isFillRect = pAnnotDict->GetArrayBy("IC") && Can we save the pointer instead of calling pAnnotDict->GetArrayBy("IC") thrice?
https://codereview.chromium.org/2219683002/diff/1/core/fpdfdoc/cpvt_generatea... File core/fpdfdoc/cpvt_generateap.cpp (right): https://codereview.chromium.org/2219683002/diff/1/core/fpdfdoc/cpvt_generatea... core/fpdfdoc/cpvt_generateap.cpp:455: CFX_ByteString GetColorStringWithDefault(CPDF_Array* pColor, On 2016/08/05 02:01:05, Lei Zhang wrote: > What prompted this change? Seemed fine before. For Square annotation, stroke color is in "C" field, and fill color is in "IC" field. So I wanted to use this function to get the color string for both stroke and fill color. https://codereview.chromium.org/2219683002/diff/1/core/fpdfdoc/cpvt_generatea... core/fpdfdoc/cpvt_generateap.cpp:469: return pBorderStyleDict->GetNumberBy("W"); On 2016/08/05 02:01:05, Lei Zhang wrote: > Do you care if this or line 473 returns 0? If a malformed PDF returns a negative > number, is that ok? It can be 0, in which case the border will not be drawn. However, if malformed PDF returns a negative number, should I return 0 or the default value, which is 1? Returning default value for now. https://codereview.chromium.org/2219683002/diff/1/core/fpdfdoc/cpvt_generatea... core/fpdfdoc/cpvt_generateap.cpp:478: CFX_ByteString GetDashArrayInString(const CPDF_Dictionary& pAnnotDict) { On 2016/08/05 02:01:05, Lei Zhang wrote: > just GetDashArrayString? Done. https://codereview.chromium.org/2219683002/diff/1/core/fpdfdoc/cpvt_generatea... core/fpdfdoc/cpvt_generateap.cpp:491: sDashStream << "[" << pDashArray->GetNumberAt(0); On 2016/08/05 02:01:05, Lei Zhang wrote: > If |pDashArray| is empty, you would get "[ 0 ]", is that ok? I changed it to return empty string when |pDashArray| is empty. https://codereview.chromium.org/2219683002/diff/1/core/fpdfdoc/cpvt_generatea... core/fpdfdoc/cpvt_generateap.cpp:668: bool isFillRect = pAnnotDict->GetArrayBy("IC") && On 2016/08/05 02:01:05, Lei Zhang wrote: > Can we save the pointer instead of calling pAnnotDict->GetArrayBy("IC") thrice? Done. https://codereview.chromium.org/2219683002/diff/1/core/fpdfdoc/cpvt_generatea... core/fpdfdoc/cpvt_generateap.cpp:668: bool isFillRect = pAnnotDict->GetArrayBy("IC") && On 2016/08/05 02:01:05, Lei Zhang wrote: > bIsFillRect for consistency. Done.
https://codereview.chromium.org/2219683002/diff/1/core/fpdfdoc/cpvt_generatea... File core/fpdfdoc/cpvt_generateap.cpp (right): https://codereview.chromium.org/2219683002/diff/1/core/fpdfdoc/cpvt_generatea... core/fpdfdoc/cpvt_generateap.cpp:469: return pBorderStyleDict->GetNumberBy("W"); On 2016/08/05 02:49:37, jaepark wrote: > On 2016/08/05 02:01:05, Lei Zhang wrote: > > Do you care if this or line 473 returns 0? If a malformed PDF returns a > negative > > number, is that ok? > > It can be 0, in which case the border will not be drawn. However, if malformed > PDF returns a negative number, should I return 0 or the default value, which is > 1? Returning default value for now. Why don't we generate a test PDF and find out?
https://codereview.chromium.org/2219683002/diff/1/core/fpdfdoc/cpvt_generatea... File core/fpdfdoc/cpvt_generateap.cpp (right): https://codereview.chromium.org/2219683002/diff/1/core/fpdfdoc/cpvt_generatea... core/fpdfdoc/cpvt_generateap.cpp:455: CFX_ByteString GetColorStringWithDefault(CPDF_Array* pColor, On 2016/08/05 02:49:37, jaepark wrote: > On 2016/08/05 02:01:05, Lei Zhang wrote: > > What prompted this change? Seemed fine before. > > For Square annotation, stroke color is in "C" field, and fill color is in "IC" > field. So I wanted to use this function to get the color string for both stroke > and fill color. Another option is to change this to: enum class ColorType { COLOR, INTERIOR_COLOR, }; GetColorStringWithDefault(const CPDF_Dictionary& pAnnotDict, ColorType color, ...); Whichever you think is cleaner.
Also, updated test cases with dash lines in https://codereview.chromium.org/2216193002 . https://codereview.chromium.org/2219683002/diff/1/core/fpdfdoc/cpvt_generatea... File core/fpdfdoc/cpvt_generateap.cpp (right): https://codereview.chromium.org/2219683002/diff/1/core/fpdfdoc/cpvt_generatea... core/fpdfdoc/cpvt_generateap.cpp:455: CFX_ByteString GetColorStringWithDefault(CPDF_Array* pColor, On 2016/08/05 17:27:29, Lei Zhang wrote: > On 2016/08/05 02:49:37, jaepark wrote: > > On 2016/08/05 02:01:05, Lei Zhang wrote: > > > What prompted this change? Seemed fine before. > > > > For Square annotation, stroke color is in "C" field, and fill color is in "IC" > > field. So I wanted to use this function to get the color string for both > stroke > > and fill color. > > Another option is to change this to: > > enum class ColorType { > COLOR, > INTERIOR_COLOR, > }; > > GetColorStringWithDefault(const CPDF_Dictionary& pAnnotDict, ColorType color, > ...); > > Whichever you think is cleaner. Acknowledged. https://codereview.chromium.org/2219683002/diff/1/core/fpdfdoc/cpvt_generatea... core/fpdfdoc/cpvt_generateap.cpp:469: return pBorderStyleDict->GetNumberBy("W"); On 2016/08/05 15:50:09, Lei Zhang wrote: > On 2016/08/05 02:49:37, jaepark wrote: > > On 2016/08/05 02:01:05, Lei Zhang wrote: > > > Do you care if this or line 473 returns 0? If a malformed PDF returns a > > negative > > > number, is that ok? > > > > It can be 0, in which case the border will not be drawn. However, if malformed > > PDF returns a negative number, should I return 0 or the default value, which > is > > 1? Returning default value for now. > > Why don't we generate a test PDF and find out? Figured out that if the border width is 0 or negative number, the border should not be drawn.
https://codereview.chromium.org/2219683002/diff/40001/core/fpdfdoc/cpvt_gener... File core/fpdfdoc/cpvt_generateap.cpp (right): https://codereview.chromium.org/2219683002/diff/40001/core/fpdfdoc/cpvt_gener... core/fpdfdoc/cpvt_generateap.cpp:486: } else if (CPDF_Array* pBorderArray = pAnnotDict.GetArrayBy("Border")) { In GetBorderWidth() above, it checks the "BS" dict first, and if it exists but does not have a "W" key, then it tries the "Border" array next. Here, the function checks the "BS" dict first, and if it exists but does not have a "S" key, then it does not try the "Border" array next. Should the two be consistent? https://codereview.chromium.org/2219683002/diff/40001/core/fpdfdoc/cpvt_gener... core/fpdfdoc/cpvt_generateap.cpp:555: CFX_ByteString GetPaintOperatorString(bool bIsStrokeRect, bool bIsFillRect) { If you want it to be more compact: if (bIsStrokeRect) return bIsFillRect ? "b" : "s"; return bIsFillRect ? "f" : "n"; https://codereview.chromium.org/2219683002/diff/40001/core/fpdfdoc/cpvt_gener... core/fpdfdoc/cpvt_generateap.cpp:558: else if (bIsStrokeRect) no else after return https://codereview.chromium.org/2219683002/diff/40001/core/fpdfdoc/cpvt_gener... core/fpdfdoc/cpvt_generateap.cpp:696: sAppStream << fBorderWidth << " w "; So we write into the AP stream, even if the width is 0 or negative?
Description was changed from ========== Generate default AP stream for square annotation. This patch generates a default AP stream for square annotation so that square annotations without AP stream can be displayed. BUG=62625 ========== to ========== Generate default AP stream for square annotation. This patch generates a default AP stream for square annotation so that square annotations without AP stream can be displayed. Also, roll DEPS for testing/corpus to 9a13509 to test square annotations. BUG=62625 ==========
The CQ bit was checked by jaepark@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_no_v8 on master.tryserver.client.pdfium (JOB_FAILED, https://build.chromium.org/p/tryserver.client.pdfium/builders/linux_no_v8/bui...)
Dry bots are failing because of a corpus test case with square annotation that was not drawn previously. I've updated the expected file in https://codereview.chromium.org/2219753003/ . https://codereview.chromium.org/2219683002/diff/40001/core/fpdfdoc/cpvt_gener... File core/fpdfdoc/cpvt_generateap.cpp (right): https://codereview.chromium.org/2219683002/diff/40001/core/fpdfdoc/cpvt_gener... core/fpdfdoc/cpvt_generateap.cpp:486: } else if (CPDF_Array* pBorderArray = pAnnotDict.GetArrayBy("Border")) { On 2016/08/05 22:14:45, Lei Zhang wrote: > In GetBorderWidth() above, it checks the "BS" dict first, and if it exists but > does not have a "W" key, then it tries the "Border" array next. > > Here, the function checks the "BS" dict first, and if it exists but does not > have a "S" key, then it does not try the "Border" array next. > > Should the two be consistent? Done. https://codereview.chromium.org/2219683002/diff/40001/core/fpdfdoc/cpvt_gener... core/fpdfdoc/cpvt_generateap.cpp:555: CFX_ByteString GetPaintOperatorString(bool bIsStrokeRect, bool bIsFillRect) { On 2016/08/05 22:14:45, Lei Zhang wrote: > If you want it to be more compact: > > if (bIsStrokeRect) > return bIsFillRect ? "b" : "s"; > return bIsFillRect ? "f" : "n"; Done. https://codereview.chromium.org/2219683002/diff/40001/core/fpdfdoc/cpvt_gener... core/fpdfdoc/cpvt_generateap.cpp:558: else if (bIsStrokeRect) On 2016/08/05 22:14:45, Lei Zhang wrote: > no else after return Done. https://codereview.chromium.org/2219683002/diff/40001/core/fpdfdoc/cpvt_gener... core/fpdfdoc/cpvt_generateap.cpp:696: sAppStream << fBorderWidth << " w "; On 2016/08/05 22:14:45, Lei Zhang wrote: > So we write into the AP stream, even if the width is 0 or negative? Whether to stroke or not depends on the paint operator at the end of the path stream. Even if the width is not set, it will draw with default width if we use "s" or "b" operator. Also, line width "0" means draw with thinnest line possible. So to prevent from stroking, we have to use "n" paint operator. Do you think it's better to not set the width when |fBorderWidth| is less than or equal to 0?
https://codereview.chromium.org/2219683002/diff/40001/core/fpdfdoc/cpvt_gener... File core/fpdfdoc/cpvt_generateap.cpp (right): https://codereview.chromium.org/2219683002/diff/40001/core/fpdfdoc/cpvt_gener... core/fpdfdoc/cpvt_generateap.cpp:696: sAppStream << fBorderWidth << " w "; On 2016/08/05 23:07:39, jaepark wrote: > On 2016/08/05 22:14:45, Lei Zhang wrote: > > So we write into the AP stream, even if the width is 0 or negative? > > Whether to stroke or not depends on the paint operator at the end of the path > stream. Even if the width is not set, it will draw with default width if we use > "s" or "b" operator. Also, line width "0" means draw with thinnest line > possible. So to prevent from stroking, we have to use "n" paint operator. Do you > think it's better to not set the width when |fBorderWidth| is less than or equal > to 0? I'm just want to make sure we render correctly if we get a PDF with a negative width. As another example, negative width means rect.Deflate() will actually inflate. https://codereview.chromium.org/2219683002/diff/60001/core/fpdfdoc/cpvt_gener... File core/fpdfdoc/cpvt_generateap.cpp (right): https://codereview.chromium.org/2219683002/diff/60001/core/fpdfdoc/cpvt_gener... core/fpdfdoc/cpvt_generateap.cpp:490: pDashArray = pBorderStyleDict->GetArrayBy("D"); Now "BS" has priority over "Border" if they both exist.
Need to roll DEPS after landing updated expected result for negative border width https://codereview.chromium.org/2223103002 . https://codereview.chromium.org/2219683002/diff/40001/core/fpdfdoc/cpvt_gener... File core/fpdfdoc/cpvt_generateap.cpp (right): https://codereview.chromium.org/2219683002/diff/40001/core/fpdfdoc/cpvt_gener... core/fpdfdoc/cpvt_generateap.cpp:696: sAppStream << fBorderWidth << " w "; On 2016/08/06 00:10:37, Lei Zhang wrote: > I'm just want to make sure we render correctly if we get a PDF with a negative > width. As another example, negative width means rect.Deflate() will actually > inflate. I've changed it to not add the line width string and dash line string when the line width is 0 or negative. I've also modified it so that the rect does not deflate when the line width is 0 or negative. This causes a change in testing/corpus test result for negative line width (https://codereview.chromium.org/2223103002). https://codereview.chromium.org/2219683002/diff/60001/core/fpdfdoc/cpvt_gener... File core/fpdfdoc/cpvt_generateap.cpp (right): https://codereview.chromium.org/2219683002/diff/60001/core/fpdfdoc/cpvt_gener... core/fpdfdoc/cpvt_generateap.cpp:490: pDashArray = pBorderStyleDict->GetArrayBy("D"); On 2016/08/06 00:10:37, Lei Zhang wrote: > Now "BS" has priority over "Border" if they both exist. I might be mistaken, but isn't this what we wanted? For GetBorderWidth, "BS" also has priority over "Border" if they both exist.
https://codereview.chromium.org/2219683002/diff/60001/core/fpdfdoc/cpvt_gener... File core/fpdfdoc/cpvt_generateap.cpp (right): https://codereview.chromium.org/2219683002/diff/60001/core/fpdfdoc/cpvt_gener... core/fpdfdoc/cpvt_generateap.cpp:490: pDashArray = pBorderStyleDict->GetArrayBy("D"); On 2016/08/08 17:17:27, jaepark wrote: > On 2016/08/06 00:10:37, Lei Zhang wrote: > > Now "BS" has priority over "Border" if they both exist. > > I might be mistaken, but isn't this what we wanted? For GetBorderWidth, "BS" > also has priority over "Border" if they both exist. Oh, I missed the part where you checked "Border" first here. So this function checks "Border" first and throws away the result if "BS" has the entry with priority. Can we instead write: CPDF_Array* pDashArray = nullptr; // Check "BS" if (!pDashArray) { // Check "Border" } so it does not have to check both dictionaries if they are both present?
Description was changed from ========== Generate default AP stream for square annotation. This patch generates a default AP stream for square annotation so that square annotations without AP stream can be displayed. Also, roll DEPS for testing/corpus to 9a13509 to test square annotations. BUG=62625 ========== to ========== Generate default AP stream for square annotation. This patch generates a default AP stream for square annotation so that square annotations without AP stream can be displayed. Also, roll DEPS for testing/corpus to 7f07c22 to test square annotations. BUG=62625 ==========
The CQ bit was checked by jaepark@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2219683002/diff/60001/core/fpdfdoc/cpvt_gener... File core/fpdfdoc/cpvt_generateap.cpp (right): https://codereview.chromium.org/2219683002/diff/60001/core/fpdfdoc/cpvt_gener... core/fpdfdoc/cpvt_generateap.cpp:490: pDashArray = pBorderStyleDict->GetArrayBy("D"); On 2016/08/08 22:24:45, Lei Zhang wrote: > On 2016/08/08 17:17:27, jaepark wrote: > > On 2016/08/06 00:10:37, Lei Zhang wrote: > > > Now "BS" has priority over "Border" if they both exist. > > > > I might be mistaken, but isn't this what we wanted? For GetBorderWidth, "BS" > > also has priority over "Border" if they both exist. > > Oh, I missed the part where you checked "Border" first here. > > So this function checks "Border" first and throws away the result if "BS" has > the entry with priority. Can we instead write: > > CPDF_Array* pDashArray = nullptr; > // Check "BS" > if (!pDashArray) { > // Check "Border" > } > > so it does not have to check both dictionaries if they are both present? I've modified it to use GetDashArray, a new function similar to GetBorderWidth. How's this?
Patch set 6 works for me. https://codereview.chromium.org/2219683002/diff/100001/core/fpdfdoc/cpvt_gene... File core/fpdfdoc/cpvt_generateap.cpp (right): https://codereview.chromium.org/2219683002/diff/100001/core/fpdfdoc/cpvt_gene... core/fpdfdoc/cpvt_generateap.cpp:500: if (pDashArrayCount == 0 || pDashArrayCount > 10) BTW, do you want to make a sample PDF where the array has more than 10 elements, so we can see if other PDF viewers drop the entire array or truncate at 10?
The CQ bit was checked by jaepark@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2219683002/diff/100001/core/fpdfdoc/cpvt_gene... File core/fpdfdoc/cpvt_generateap.cpp (right): https://codereview.chromium.org/2219683002/diff/100001/core/fpdfdoc/cpvt_gene... core/fpdfdoc/cpvt_generateap.cpp:500: if (pDashArrayCount == 0 || pDashArrayCount > 10) On 2016/08/08 23:10:41, Lei Zhang wrote: > BTW, do you want to make a sample PDF where the array has more than 10 elements, > so we can see if other PDF viewers drop the entire array or truncate at 10? I've changed it to truncate at 10.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm https://codereview.chromium.org/2219683002/diff/120001/core/fpdfdoc/cpvt_gene... File core/fpdfdoc/cpvt_generateap.cpp (right): https://codereview.chromium.org/2219683002/diff/120001/core/fpdfdoc/cpvt_gene... core/fpdfdoc/cpvt_generateap.cpp:501: pDashArray->GetCount() < 10 ? pDashArray->GetCount() : 10; std::min(pDashArray->GetCount(), 10U);
The CQ bit was checked by jaepark@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_no_v8 on master.tryserver.client.pdfium (JOB_FAILED, https://build.chromium.org/p/tryserver.client.pdfium/builders/win_no_v8/build...) win_xfa on master.tryserver.client.pdfium (JOB_FAILED, https://build.chromium.org/p/tryserver.client.pdfium/builders/win_xfa/builds/...)
https://codereview.chromium.org/2219683002/diff/120001/core/fpdfdoc/cpvt_gene... File core/fpdfdoc/cpvt_generateap.cpp (right): https://codereview.chromium.org/2219683002/diff/120001/core/fpdfdoc/cpvt_gene... core/fpdfdoc/cpvt_generateap.cpp:501: pDashArray->GetCount() < 10 ? pDashArray->GetCount() : 10; On 2016/08/09 00:44:31, Lei Zhang wrote: > std::min(pDashArray->GetCount(), 10U); Using std::min(pDashArray->GetCount(), 10UL) seems to be failing in windows build. Do you think using static_cast to use std::min() is a better approach?
Sigh, Windows. https://codereview.chromium.org/2219683002/diff/120001/core/fpdfdoc/cpvt_gene... File core/fpdfdoc/cpvt_generateap.cpp (right): https://codereview.chromium.org/2219683002/diff/120001/core/fpdfdoc/cpvt_gene... core/fpdfdoc/cpvt_generateap.cpp:501: pDashArray->GetCount() < 10 ? pDashArray->GetCount() : 10; On 2016/08/09 02:34:16, jaepark wrote: > On 2016/08/09 00:44:31, Lei Zhang wrote: > > std::min(pDashArray->GetCount(), 10U); > > Using std::min(pDashArray->GetCount(), 10UL) seems to be failing in windows > build. Do you think using static_cast to use std::min() is a better approach? Try: std::min<size_t>(pDashArray->GetCount(), 10);
The CQ bit was checked by jaepark@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2219683002/diff/120001/core/fpdfdoc/cpvt_gene... File core/fpdfdoc/cpvt_generateap.cpp (right): https://codereview.chromium.org/2219683002/diff/120001/core/fpdfdoc/cpvt_gene... core/fpdfdoc/cpvt_generateap.cpp:501: pDashArray->GetCount() < 10 ? pDashArray->GetCount() : 10; On 2016/08/09 05:04:46, Lei Zhang wrote: > On 2016/08/09 02:34:16, jaepark wrote: > > On 2016/08/09 00:44:31, Lei Zhang wrote: > > > std::min(pDashArray->GetCount(), 10U); > > > > Using std::min(pDashArray->GetCount(), 10UL) seems to be failing in windows > > build. Do you think using static_cast to use std::min() is a better approach? > > Try: std::min<size_t>(pDashArray->GetCount(), 10); Done.
The CQ bit was checked by jaepark@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from thestig@chromium.org Link to the patchset: https://codereview.chromium.org/2219683002/#ps160001 (title: "Generate default AP stream for square annotation.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Generate default AP stream for square annotation. This patch generates a default AP stream for square annotation so that square annotations without AP stream can be displayed. Also, roll DEPS for testing/corpus to 7f07c22 to test square annotations. BUG=62625 ========== to ========== Generate default AP stream for square annotation. This patch generates a default AP stream for square annotation so that square annotations without AP stream can be displayed. Also, roll DEPS for testing/corpus to 7f07c22 to test square annotations. BUG=62625 Committed: https://pdfium.googlesource.com/pdfium/+/af7ab33c73f58f18d0db0c90d93fa0aab0bc... ==========
Message was sent while issue was closed.
Committed patchset #9 (id:160001) as https://pdfium.googlesource.com/pdfium/+/af7ab33c73f58f18d0db0c90d93fa0aab0bc... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
