Annotations: refactor setting the normal appearance stream #8065
Conversation
|
/botio-linux preview |
|
/botio test |
There was a problem hiding this comment.
As can be seen from the test results, this causes "double" rendering in some of the annotation tests.
I'd assume that the source of the problems is that you're now attempting to send a Stream (compared to just a boolean previously) from the worker to the main thread, which most likely is not going to work since its data structure is too complicated for that.
Edit: Basically, you need to revert the second commit, since it first of all doesn't work to send a Stream to the display layer and second of all there's nothing you can use it for there (even if sending it would work).
timvandermeij
force-pushed
the
annotation-appearances
branch
from
f6c2d5d to
64e797d
Compare
timvandermeij
changed the title
hasAppearance
timvandermeij
force-pushed
the
annotation-appearances
branch
from
64e797d to
123a196
Compare
|
Thank you for pointing this out! I removed the second commit and improved the first commit with a comment above the method (which I initially forgot). Hopefully all is fine now. Without that second commit, there are no more API changes in this patch. |
|
/botio-linux preview |
|
/botio test |
src/core/annotation.js
Outdated
| if (isStream(normalAppearanceState)) { | ||
| this.appearance = normalAppearanceState; | ||
| return; | ||
| } |
There was a problem hiding this comment.
I'm thinking that you may be able to simplify the conditions above slightly.
Wouldn't the following code work just as well?
// In case the normal appearance is a stream, then it is used directly.
if (isStream(normalAppearanceState)) {
this.appearance = normalAppearanceState;
return;
}
if (!isDict(normalAppearanceState)) {
return;
}
// In case the normal appearance is a dictionary, the `AS` entry provides
// the key of the stream in this dictionary.
...There was a problem hiding this comment.
Good point. This reduced the length of the method by two lines.
Previously, we had a function called `getDefaultAppearance`. This name, however, is misleading as the method gets the normal appearance (in the `N` entry) and not the default appearance (in the `DA` entry). Moreover, it was not entirely clear how it works just from reading the code. It primarily lacks comments and explicit error case handling. This patch improves the situation by fixing the issues mentioned above and making this function a proper method of the `Annotation` class, just like e.g., `setColor` and `setBorderStyle`.
timvandermeij
force-pushed
the
annotation-appearances
branch
from
123a196 to
26fc79d
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/22bcb7ed291afbc/output.txt |
From: Bot.io (Linux)SuccessFull output at http://107.21.233.14:8877/22bcb7ed291afbc/output.txt Total script time: 2.18 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/48d06f15c39e582/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @timvandermeij received. Current queue size: 0 Live output at: http://54.215.176.217:8877/6fe154a92c1271c/output.txt |
From: Bot.io (Windows)SuccessFull output at http://54.215.176.217:8877/6fe154a92c1271c/output.txt Total script time: 21.49 mins
|
From: Bot.io (Linux)SuccessFull output at http://107.21.233.14:8877/48d06f15c39e582/output.txt Total script time: 25.62 mins
|
…nces Annotations: refactor setting the normal appearance stream
Previously, we had a function called
getDefaultAppearance. This name, however, is misleading as the method gets the normal appearance (in theNentry) and not the default appearance (in theDAentry). Moreover, it was not entirely clear how it works just from reading the code. It primarily lacks comments and explicit error case handling.This patch improves the situation by fixing the issues mentioned above and making this function a proper method of the
Annotationclass, just like e.g.,setColorandsetBorderStyle.