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

Import Image UI (allows for importing via URLs and by local image file #30

Conversation

ChronoAndross
Copy link

The code changes that are made here are as follows:

  1. I added a new file named ImportModal.js.
  2. I added a new main method called loadImageFile, which essentially parses the entirety of the current shader and adds the appropriate path next to the new texture name. I call setUniforms for local files since the image comes in as a data64 string.
  3. Added an instance of ImportModal class to the Menu class using a similar pattern used for the ExportModal class.

NOTE: This does not capture CORS requests if they occur in these instances. I was going to attempt to try this, but the response for CORS happens to far in the call stack to catch it. Perhaps I'll work on this at some other point.

For more information about the behavior of this new feature, please look at my latest comment in this issue: #29

Thanks!

Andrew Gorbaty added 3 commits January 29, 2018 23:50
…textures within the current shader. NOTE: THIS IS NOT FINISHED YET
…path of the image to loadImageFile main method.\n2.Refactored selecting options for both getting local/URL files.\n3.Created new error handling cases, including checking if currently in Chrome if attempting to import a local file, and if the image is not an image file for importing images via URLs.\n\nTODO: Catch CORS failures since many images on servers fail this test.
@patriciogonzalezvivo
Copy link
Owner

thanks for the patience @ChronoAndross. Although loadImageFile using base64 it's a great idea to keep shaders compact and independent, it relays on the space on my server which I pay only with donations. That's a pattern that I don't wish to promote. Do you mind taking out that feature?

@ChronoAndross
Copy link
Author

@patriciogonzalezvivo I can take out that feature and just accept paste the local file path and submit that today.

Thanks for getting back to me!

@ChronoAndross
Copy link
Author

ChronoAndross commented Mar 3, 2018

Actually, now I'm looking back at my code, I know why I did it this way to begin with. I did it this way since the input tag won't give me a full local file path by default because of security reasons, and I don't think I can get around this. Should I just disable the feature, or should I just change the UI so that the user gets the same dialog when importing a URL image?

Thanks!

… we can cut down on the size of the shader. Also took out anything concerning data64 as input upon request. WARNING: The texture won't show upon local import this way since the Texture.js code keeps rejecting the local URI. This has to be changed from OP's side since that code is not presently in the repo. This doesn't even work in the present builds of glslEditor.
@ChronoAndross
Copy link
Author

Okay so I've changed my code to just paste the local path, but local imports don't presently work since the Texture.js code keeps rejecting local URIs. I can't directly change this code. @patriciogonzalezvivo is this something you can do? I'm betting it has something to do with loading the local image data. This is why I loaded the data using base64 since a base64 stream can be read locally fairly easily.

Thanks!

@patriciogonzalezvivo
Copy link
Owner

Yes using URL to the cloud (checking previously that don't raise CORS issues) should be the only way to import an image. Why?

  • local files wont work for other users
  • base64 encoded images, promote "sharing" the image in the shader and that drastically increment the size of the shader... and that potentially will collapse the server and/or make me pay extra $$$... which I'm not willing to do.

For then, remotes URL with no CORS issues should be the only available option. People need to host their own images using dropbox or another services

@patriciogonzalezvivo patriciogonzalezvivo deleted the branch patriciogonzalezvivo:updated_not_working October 30, 2022 16:55
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.

2 participants