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

remove giu.Texture example; use image.RGBA instead #333

Open
gucio321 opened this issue Jul 22, 2021 · 7 comments
Open

remove giu.Texture example; use image.RGBA instead #333

gucio321 opened this issue Jul 22, 2021 · 7 comments

Comments

@gucio321
Copy link
Contributor

Hi there,
we can remove/reduce texture loading stuff as it isn't really necessary.
for images (in dcc/dc6/dt1 editors) we can use ImageWithRGBA instead of Image.
at the moment, no idea for image buttons, I opened an issue: AllenDang/giu#233
the final step, could be removing TextureLoader as it'll not be necessary anymore.

@gucio321
Copy link
Contributor Author

I can do that.

@gravestench
Copy link
Contributor

From your description above, I'm not sure what the issue is.

What is the issue?

@gucio321
Copy link
Contributor Author

hmm, okey there is few issues, i see with texture loader.

  1. everyone, who wants to use our widgets, needs to pass TextureLoader there, I don't think it is ok,
    just like: why user should care, how the widget loads its textures?
  2. it isn't necessary to load textures with textureLoader for images (in dcc/dc6/dt1 editors). we're creating image.RGBA and then, converting to giu.Texture. but at the moment giu has ImageWithRgbaWidget so we don't need to do that.

so, in summary, there is a lot of code, we can clean up and simplify.

@gravestench
Copy link
Contributor

But you're actually proposing multiple refactors:

  1. Remove hscommon.TextureLoader from widget provider functions (this kind of detail should be hidden from external callers, makes sense)
  2. Change image implementation used by widgets (to reduce complexity, good idea)
  3. Remove hscommon.TextureLoader entirely from codebase (make loading textures less painful, I like it)

If this is the case, I'd like for you to close this issue and open individual issues, and to provide coherent titles and descriptions for those issues.

Thanks!

@gucio321
Copy link
Contributor Author

yah, thats the case, but these points are strongly related with each other. you can't Remove hscommon.TextureLoader from widget provider functions before you Change image implementation used by widgets (images and imageButtons btw).
if you wish, i can open as separated issues, but we will still have to implement them in a specific order.

@gravestench
Copy link
Contributor

gravestench commented Jul 22, 2021

you can't Remove hscommon.TextureLoader from widget provider functions before you Change image implementation used by widgets

I'd appreciate it if you provided that kind of detail when you open an issue.

Also, it sounds like that wouldn't be an issue if we did another refactor that unifies the way we load images throughout the entire codebase (I thought this is what hscommon.TextureLoader was doing.)

@gucio321
Copy link
Contributor Author

okey, after a few tries, I came to the conclusion that removing giu.Textures isn't so easy.
the reason is:
imageWithRGBAWidget has bug/feature that causes losing image texture when image isn't visible. So in dcc/dc6 editors we'll se a kind of delay while scrolling around frames/directions. It'd be better to preload textures and show them by ImageWidget

in case of TextureLoader: I'm not sure it is necessary. Does anyone remember why don't we use giu.NewTextureFromRgba(...)?

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

No branches or pull requests

2 participants