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

16-bit PNG and raw image support #100

Merged
merged 4 commits into from
Jul 9, 2019
Merged

Conversation

perlman
Copy link
Collaborator

@perlman perlman commented Jun 24, 2019

I am reviving a long overdue thread for better png support (#76).

As of now, this branch adds a "png16-image" target, as well as "raw-image" (RGBA) and "raw16-image) (short gray), which I'm using for small experiment at the moment.

@trautmane: Instead of creating end-points for 16-bit, do you think the data type should be specified as an argument, e.g. &dtype=RGBA or &grayscale=true&depth=16 or something like that?

@perlman perlman requested a review from trautmane June 24, 2019 18:56
@perlman perlman self-assigned this Jun 24, 2019
@trautmane
Copy link
Collaborator

Hi Eric, Apologies for not looking at this sooner. Your distinct end-points vs. query parameters question is one that I have struggled with for a while. My original (possibly now outdated) rationale for creating distinct end-points for the image APIs was to ensure they were cache friendly. Years ago some caches ignored query parameters. Even today, I think most caches are sensitive to query parameter ordering (e.g. ...grayscale=true&depth=16... and ...depth=16&grayscale=true... reference the same image but get cached separately). From a code standpoint, I think the use of query parameters is much more intuitive and maintainable. Every time we introduce another image variant, I am enticed to move to the query parameter style. Since you've already gone through the (mild) pain of adding distinct end-points here, let's keep what you've done. I'm happy to consider/discuss alternatives if you have other thoughts on the subject.

@trautmane
Copy link
Collaborator

Eric - I merged upstream changes and fixed one typo on a local copy of your branch. Can you allow me to commit these changes to your repo ( see https://help.github.com/en/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork ) so that I can easily merge this pull request with my fixes?

@perlman
Copy link
Collaborator Author

perlman commented Jul 9, 2019

"Allow edits from maintainers." is already checked.

These changes would be immediately useful for me to PR 16-bit support to neuroglancer, so, thanks! I will plan on adding arguments to reduce the number of endpoint permutations. The png & raw endpoints should be able to handle all byte, short, RGB and RGBA with a small amount of work.

@trautmane trautmane merged commit 8a8349b into saalfeldlab:master Jul 9, 2019
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.

2 participants