Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ISSUE-203:OpenCV is the new Black #204

Merged
merged 2 commits into from
Jun 27, 2022
Merged

ISSUE-203:OpenCV is the new Black #204

merged 2 commits into from
Jun 27, 2022

Conversation

DiegoPino
Copy link
Member

What?

See #203

Leaving this here and getting a coffee

image

This makes now main load of pages slimmer, adds new openCV worker + utils + UX for annotations and even some cool SVG icons
@DiegoPino DiegoPino added enhancement New feature or request Javascript Favourite language of a PHP developer UI driven hints Show stuff on screen so people have less guessing to do UX Like UI but with an X Annotation API Paint and scratch, share your feelings with SVG labels Jun 3, 2022
@DiegoPino DiegoPino added this to the 1.0.0 milestone Jun 3, 2022
@DiegoPino DiegoPino self-assigned this Jun 3, 2022
@aksm
Copy link
Contributor

aksm commented Jun 6, 2022

@DiegoPino Just got this running on a local instance. Will begin reviewing the code now.

Copy link
Contributor

@aksm aksm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't review too closely since I don't want to delay merging, but testing functionality didn't reveal any major errors. @DiegoPino probably you've already noticed this, but the FS error goes away when I add a breakpoint and resume so looks like some kind of race condition? Will continue to debug to see if I can figure anything out.

- core/drupal
- core/drupalSettings
- format_strawberryfield/iiif_openseadragon
- format_strawberryfield/annotoriousopenseadragon
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- format_strawberryfield/annotoriousopenseadragon
- format_strawberryfield/annotoriousopenseadragon
- format_strawberryfield/opencv

Adding opencv as a dependency seems to get rid of the FS error in my testing.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aksm can you open tomorrow another issue about adding opencv as a dependency + maybe some extra small fixes? (e.g now with the tool bar the whole OSD height overlays with the other field formatters). Reason I do not want to go blindly with this is because OpenCV is 8 Mbytes in size, so making it a dependency will force it to load synchronously instead via the Web Worker (background)

@DiegoPino DiegoPino merged commit 51cd6e6 into 1.0.0 Jun 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Annotation API Paint and scratch, share your feelings with SVG enhancement New feature or request Javascript Favourite language of a PHP developer UI driven hints Show stuff on screen so people have less guessing to do UX Like UI but with an X
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants