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

Add subcube loading #320

Open
wants to merge 51 commits into
base: main
Choose a base branch
from
Open

Add subcube loading #320

wants to merge 51 commits into from

Conversation

Pietersielie
Copy link
Collaborator

@Pietersielie Pietersielie commented Aug 29, 2023

Resolve #307 and resolve #318.

Pietersielie and others added 8 commits June 6, 2023 14:45
…r a triplet set or configuration that is not there.
…Issue with toggle and input field not allowing interaction.
…to the other interactables of the subset UI.� * Added subset loading of fits files to VolumeDataSet.

  * Added subset loading of mask to fits_reader c++ library.
  * Fix an issue where the UI subset assumed zero-indexed.
  * Fix that the smallest dimension was two, it's now one.
  * Fix that the UI allowed max to be less than min and vice versa.
  * Cell outline is now broken, need to fix.
@Pietersielie Pietersielie added enhancement New feature or request GUI Unity GUI work required labels Aug 29, 2023
@Pietersielie Pietersielie added this to the V 1.0 GUI milestone Aug 29, 2023
@Pietersielie Pietersielie marked this pull request as draft August 31, 2023 07:29
@Pietersielie
Copy link
Collaborator Author

Add crop select and values to the sub cubes.

Copy link
Collaborator

@CosmicElysium CosmicElysium left a comment

Choose a reason for hiding this comment

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

Code looks good. Just some removal of old commented out stuff. I think we wait a little on testing before merging.

Comment on lines 203 to 214
// savePopup.transform.SetParent(this.transform.parent, false);
// savePopup.transform.localPosition = this.transform.localPosition;
// savePopup.transform.localRotation = this.transform.localRotation;
// savePopup.transform.localScale = this.transform.localScale;

// savePopup.transform.Find("Content").gameObject.transform.Find("FirstRow").gameObject.transform.Find("Cancel").GetComponent<Button>().onClick.RemoveAllListeners();
// savePopup.transform.Find("Content").gameObject.transform.Find("FirstRow").gameObject.transform.Find("Overwrite").GetComponent<Button>().onClick.RemoveAllListeners();
// savePopup.transform.Find("Content").gameObject.transform.Find("FirstRow").gameObject.transform.Find("NewFile").GetComponent<Button>().onClick.RemoveAllListeners();

// savePopup.transform.Find("Content").gameObject.transform.Find("FirstRow").gameObject.transform.Find("Cancel").GetComponent<Button>().onClick.AddListener(SaveCancel);
// savePopup.transform.Find("Content").gameObject.transform.Find("FirstRow").gameObject.transform.Find("Overwrite").GetComponent<Button>().onClick.AddListener(SaveOverwriteMask);
// savePopup.transform.Find("Content").gameObject.transform.Find("FirstRow").gameObject.transform.Find("NewFile").GetComponent<Button>().onClick.AddListener(SaveNewMask);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we just remove these lines?

Comment on lines 329 to 340
// exportPopup.transform.SetParent(this.transform.parent, false);
// exportPopup.transform.localPosition = this.transform.localPosition;
// exportPopup.transform.localRotation = this.transform.localRotation;
// exportPopup.transform.localScale = this.transform.localScale;

// exportPopup.transform.Find("Content").gameObject.transform.Find("FirstRow").gameObject.transform.Find("Cancel").GetComponent<Button>().onClick.RemoveAllListeners();
// exportPopup.transform.Find("Content").gameObject.transform.Find("FirstRow").gameObject.transform.Find("SubCube").GetComponent<Button>().onClick.RemoveAllListeners();
// exportPopup.transform.Find("Content").gameObject.transform.Find("FirstRow").gameObject.transform.Find("Mask").GetComponent<Button>().onClick.RemoveAllListeners();

// exportPopup.transform.Find("Content").gameObject.transform.Find("FirstRow").gameObject.transform.Find("Cancel").GetComponent<Button>().onClick.AddListener(ExportCancel);
// exportPopup.transform.Find("Content").gameObject.transform.Find("FirstRow").gameObject.transform.Find("SubCube").GetComponent<Button>().onClick.AddListener(SaveSubCube);
// exportPopup.transform.Find("Content").gameObject.transform.Find("FirstRow").gameObject.transform.Find("Mask").GetComponent<Button>().onClick.AddListener(SaveMask);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove these as well?

return -1;
}
std::stringstream debug;
debug << "Closing fitsfile " << fptr->Fptr->filename << ".";
Copy link
Collaborator

Choose a reason for hiding this comment

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

In the long time I have used cfitsio, I have never thought about using a member of Fptr XD

Copy link
Collaborator

Choose a reason for hiding this comment

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

Excellent! Your changes will add a bit more robustness to the fits reading/writing.

This was linked to issues Mar 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request GUI Unity GUI work required
Projects
None yet
2 participants