Skip to content

Store Meshes in Postgres #3367

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

Merged
merged 26 commits into from
Nov 22, 2018
Merged

Store Meshes in Postgres #3367

merged 26 commits into from
Nov 22, 2018

Conversation

fm3
Copy link
Member

@fm3 fm3 commented Oct 16, 2018

URL of deployed dev instance (used for testing):

@philippotto on this branch I added the new routes

POST          /api/meshes         //create
PUT           /api/meshes/:id     //update 
DELETE        /api/meshes/:id     //delete
GET           /api/meshes/:id     
PUT           /api/meshes/:id/data
GET           /api/meshes/:id/data

the data is separate from the metadata.
metadata post and put (topmost) expect a json like

{
	"annotationId": "5bc717c52601006501a87696",
	"position": [0, 0, 0],
	"description": "aMeshDescription"
}

and return a json with meshInfo.
the annotation info request contains a new field "meshes", listing the associated meshes. (the mesh metadata GET will probably not be used by the frontend)

the data requests expect/deliver raw binary data. They will fail if the metadata is not created first.

@philippotto is this how you imagined it? Please ask questions or suggest changes :)

Do you want to implement the front-end part on this branch? (Or assign someone else, of course)

Steps to test:

Issues:


@fm3 fm3 self-assigned this Oct 16, 2018
@fm3 fm3 changed the title [WIP] Store Meshes in Postgres Store Meshes in Postgres Oct 17, 2018
@fm3 fm3 added the frontend label Oct 18, 2018
@philippotto philippotto self-assigned this Nov 6, 2018
@philippotto
Copy link
Member

@fm3 I added the front-end part and think this is ready to review. I made some small changes to the back-end which you might want to double check.

Do you want to pull the master in and merge the back-end conflict? If you commit the frontend conflicts, I can resolve them then.

@philippotto philippotto requested review from daniel-wer and philippotto and removed request for philippotto November 9, 2018 10:18
@fm3
Copy link
Member Author

fm3 commented Nov 9, 2018

@philippotto that’s great, thanks!
I merged master, there are still open conflicts in app/assets/javascripts/oxalis/store.js and app/assets/javascripts/oxalis/view/layouting/default_layout_configs.js

Copy link
Member

@daniel-wer daniel-wer left a comment

Choose a reason for hiding this comment

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

Very clean implementation 👍 I like that you focused on UX and made this an easily usable feature :)

@daniel-wer
Copy link
Member

daniel-wer commented Nov 19, 2018

The feature itself works really well :) There are still some things I noticed during manual testing:

  • Initially the meshes tab is not displayed, only after reset layout, it turns up. What is the strategy for layout migrations?
  • Switching to Flight/Oblique is broken, somehow all layout panes are displayed and no data is rendered.
  • React Warning in the Console: Warning: React does not recognize the 'faIcon' prop on a DOM element. If you intentionally want it to appear in the DOM as a custom attribute, spell it as lowercase 'faicon' instead. If you accidentally passed it from a parent component, remove it from the DOM element.
  • It takes pretty long to add the stl to the scene and render it for the first time (>20s for my test unicorn). I'm not sure whether that is the same for the amira segmentation files or whether we can do anything to improve that. I did a performance measurement and the time was mostly spent in threejs.

Unicorn_20160524.zip

  • Maybe change the mouse cursor to indicate that the edit and delete icons are clickable (cursor: pointer).

@daniel-wer
Copy link
Member

I just tested again, arbitrary/flight mode works as expected, was probably related to the missing layout config version bump. :)

@fm3 fm3 merged commit 2c68652 into master Nov 22, 2018
jfrohnhofen added a commit that referenced this pull request Nov 23, 2018
* origin/master:
  Optimize performance for the list request /api/datasets (#3441)
  add annotation dataset foreign key  (#3482)
  thumbnails: correctly use zoom value if specified (#3487)
  Store Meshes in Postgres (#3367)
  fix alpha return (#3483)
  Added script to apply all new evolutions (#3427)
  Simple fix to speed up dataset gallery (#3480)
  better errors for screenshot tests, fix imports, refresh screenshots (#3479)
  (Backend only) Add project priority to progress report json (#3476)
  Handle missing write access on datastore (#3411)
  Re-introduce "Flightmode improvements"" (#3473)
  Circleci-notify: linkify PR number (#3469)
  Revert "Flightmode improvements" (#3472)
  also flow-ignore binaryData when using symlinks (#3471)
  Flightmode improvements (#3392)
  Circleci custom notification (#3465)
  enable /api/switch cross-organization (#3464)
@jstriebel jstriebel deleted the save-mesh branch November 26, 2018 09:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Store Meshes in Postgres
3 participants