Skip to content

Multiresolution image sequence support#464

Merged
subdavis merged 7 commits into
masterfrom
client/bugfix-multi-res
Dec 2, 2020
Merged

Multiresolution image sequence support#464
subdavis merged 7 commits into
masterfrom
client/bugfix-multi-res

Conversation

@subdavis
Copy link
Copy Markdown
Contributor

@subdavis subdavis commented Nov 30, 2020

fixes #55

WARNING

For multi-resolution image sequences already in the system, this will cause all detentions to become mis-aligned.

I haven't tested this extensively yet.

@subdavis subdavis requested a review from BryonLewis November 30, 2020 19:03
@subdavis subdavis changed the title Multiresolution support Multiresolution image sequence support Nov 30, 2020
Copy link
Copy Markdown
Collaborator

@BryonLewis BryonLewis left a comment

Choose a reason for hiding this comment

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

I believe that the scrubbing issue needs to be fixed at least to eliminate the errors that occur with 0 pixels width/height. I have some stashed code that I used that fixed that I can show you if you want. I didn't go through and update everything but got it so there was no more image flashes as well as no more wrong resolutions when scrubbing.

I know you can do this for your own large dataset but figured I would include for ease of testing. I just used some NOAA images, one standard one, one cropped at a tall aspect ratio and then a sea lion overhead image and then ran the following to generate (warning it could be a few GBs):

for i in {1..300}; do cp one.png "$i-one.png"; cp two.png "$i-two.png"; cp three.png "$i-three.png";  done

Comment on lines +84 to +86
const imgs = await this.cacheImages();
// reset dimensions of map from new image
const img = await imgs[this.frame];
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

My fault, I probably left in the async seek() during initial testing. I'm wondering if we really want to wait for the image to load for each seek. During testing using a large number of different sized images scrubbing using seek would stutter. This is different than waiting for the last image to load, it would get to the final position then load the last 3-4 images quickly and then the final image. I believecacheImages() has a thing in it to make sure it loaded the current frame before attempting to cache anything else (call to loadFrame). Then if the user seeked before that first frame was loaded it would invalidate the loading.

I have an idea where it may work better if within seek we only draw the frame if the image is already loaded (checking this.imgs[this.frame].cached or .complete). Then in the onload for images we check to see if the currently completed this.frame is equal the onload frame and if it is we attempt to do the resize and drawing. I think this should improve some of the seeking a bit as well as handle the different image sizes. I realize that it may warrant some more discussion.

Below is a brief clip of the PR's behavior. Note there are errors in the console as well because when seeking the current img may not be fully loaded causing it to try to reset using 0 width and height causing an error in geoViewer.bounds function.
imagecacheResize

Comment on lines +87 to +89
this.width = img.naturalWidth;
this.height = img.naturalHeight;
this.resetMapDimensions();
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I had an assumption that resizing the viewing area may be a bit taxing. Timing this out this function typically takes 1-2ms long to occur. I think we should filter either inside of this.resetMapDimensions(width, height) or elsewhere to make it so it only occurs when the width/height change for the image. I don't think we want this introducing lag on uniform image-sequences. I did testing over the EHU dataset running at 60FPS, it will lower the playback speed if trying to resetMapDimensions is called on every frame by an average of 1.5 or so ms over the full length of EHU.

Comment thread client/src/components/annotators/annotator.js
Comment thread client/src/components/annotators/annotator.js
@subdavis
Copy link
Copy Markdown
Contributor Author

subdavis commented Dec 2, 2020

  • All 90 image datasets checked.
  • Made a note of only 1 corrupted dataset that will need to be manually fixed. Low priority, little activity from user.
  • Checked datasets where data is not corrupted but annotations render wrong, will re-check again after this deploys.
  • Made requested changes.

Please review again, @BryonLewis

@subdavis subdavis force-pushed the client/bugfix-multi-res branch from b64b9ba to 7654609 Compare December 2, 2020 17:42
Copy link
Copy Markdown
Collaborator

@BryonLewis BryonLewis left a comment

Choose a reason for hiding this comment

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

Works well for seeking on large images with differing sizes. Very minor things that I noticed that don't impact much.

Comment thread client/src/components/annotators/ImageAnnotator.vue Outdated
Comment thread client/src/components/annotators/ImageAnnotator.vue Outdated
subdavis and others added 2 commits December 2, 2020 14:14
Co-authored-by: BryonLewis <61746913+BryonLewis@users.noreply.github.com>
Co-authored-by: BryonLewis <61746913+BryonLewis@users.noreply.github.com>
@subdavis subdavis merged commit 358a885 into master Dec 2, 2020
@subdavis subdavis deleted the client/bugfix-multi-res branch December 2, 2020 19:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow annotator to handle images of different resolutions in the same sequence

2 participants