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

Yuhsuan/inactive frame channel #2206

Merged
merged 4 commits into from
Aug 2, 2023
Merged

Conversation

YuHsuan-Hwang
Copy link
Collaborator

@YuHsuan-Hwang YuHsuan-Hwang commented Jul 18, 2023

Description

Closes CARTAvis/carta-python#103: setting the channel/stokes of inactive frames. Fixed by adjusting the criteria for requesting tiles when channel/stokes is manually changed.

Instead of only checking the active frame and its spectrally matched frames, we now check all visible frames and their spectrally matched frames. When checking the spectrally matched frames, we skip the visible frames to avoid duplicate requests.

Currently, users cannot set the channel/stokes of inactive frames through UI, but it is possible to do so using the code snippet feature or the console. For example:

// in multi-panel mode showing >= 2 frames
const file = await app.openFile("[path]", "[filename1]");
const file2 = await app.appendFile("[path]", "[filename2]");

// set inactive frame channel
file.setChannel(1);
// set inactive frame stokes
file.setStokes(1);

In this branch, the above code snippet will immediately request new tiles for the inactive frame. In the dev branch, it will request new tiles after the frame becomes active.

@acdo2002 There will be additional tile request messages with this change, so you might want to check it.

Checklist

For linked issues (if there are):

  • assignee and label added
  • ZenHub issue connection, board status, and estimate updated

For the pull request:

  • reviewers and assignee added
  • ZenHub estimate, milestone, and release (if needed) added
  • e2e test passing / corresponding fix added
  • changelog updated / no changelog update needed
  • protobuf updated to the latest dev commit / no protobuf update needed
  • BackendService unchanged / BackendService changed and corresponding ICD test fix added

Copy link
Collaborator

@kswang1029 kswang1029 left a comment

Choose a reason for hiding this comment

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

This should be fine. No regression from e2e tests.

Copy link
Collaborator

@confluence confluence left a comment

Choose a reason for hiding this comment

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

I can confirm that the inactive image is now re-rendered immediately when the channel is changed.

Copy link
Contributor

@acdo2002 acdo2002 left a comment

Choose a reason for hiding this comment

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

Currently the ICD-RxJS are all one file to test tiles' response.
I have confirmed this PR add additional ICD message when open two frames. ICD-RxJS need add one more tile test.

Copy link
Contributor

@I-Chenn I-Chenn left a comment

Choose a reason for hiding this comment

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

Confirm that the inactive image is rendered right after the channel is changed with my test.

@kswang1029 kswang1029 merged commit d6af83b into dev Aug 2, 2023
8 checks passed
@kswang1029 kswang1029 deleted the yuhsuan/inactive_frame_channel branch August 2, 2023 02:56
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.

raster image is not re-rendered
6 participants