Refactor annotation color handling and add unit tests #6214
Conversation
|
|
||
| 'use strict'; | ||
|
|
||
| var DEFAULT_COLOR = [0, 0, 0]; // Black in RGB color space |
There was a problem hiding this comment.
Nit: How about using new UintArray(3) here instead?
There was a problem hiding this comment.
This has been changed in the new commit so we do not even need this anymore.
timvandermeij
force-pushed
the
annotation-color
branch
from
de424d5
to
a2e9845
Compare
|
/botio-linux preview |
From: Bot.io (Linux)ReceivedCommand cmd_preview from @timvandermeij received. Current queue size: 0 Live output at: http://107.21.233.14:8877/29488461e439a6a/output.txt |
From: Bot.io (Linux)SuccessFull output at http://107.21.233.14:8877/29488461e439a6a/output.txt Total script time: 0.63 mins Published |
|
/botio test |
From: Bot.io (Linux)ReceivedCommand cmd_test from @timvandermeij received. Current queue size: 0 Live output at: http://107.21.233.14:8877/63f2826b437d469/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @timvandermeij received. Current queue size: 0 Live output at: http://107.22.172.223:8877/5a9d5b59cd9e010/output.txt |
From: Bot.io (Windows)SuccessFull output at http://107.22.172.223:8877/5a9d5b59cd9e010/output.txt Total script time: 18.30 mins
|
From: Bot.io (Linux)SuccessFull output at http://107.21.233.14:8877/63f2826b437d469/output.txt Total script time: 19.54 mins
|
Refactor annotation color handling and add unit tests
|
Looks good, thanks! |
This PR has the following goals:
setColormethod should have according to the specification at https://www.adobe.com/content/dam/Adobe/en/devnet/acrobat/pdfs/pdf_reference_1-7.pdf#page=607&zoom=auto,-246,289.setColormethod on an annotation as described by the unit tests. Note that this also adds support for grayscale and CMYK color spaces (previously a TODO) practically for free.setColor.setColortakes care of color space conversion and color array preparation (as RGB), so we do not need to convert anything in the display layer anymore.This rewrite automatically fixes an issue in the old code where transparency (here with
color = null) would not be interpreted correctly and it would draw as black instead.@Snuffleupagus Could you review this? It is a fairly easy patch and half of it are unit tests. http://tug.ctan.org/tex-archive/macros/latex/contrib/pdfcomment/doc/pdfcomment.pdf is a good document to verify that the colors are still the same before and after the patch (see page 6).