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

Texture binding + creation #112

Merged
merged 18 commits into from
Jul 20, 2014
Merged

Texture binding + creation #112

merged 18 commits into from
Jul 20, 2014

Conversation

emberian
Copy link
Contributor

Closes #49

@emberian emberian assigned kvark and unassigned kvark Jul 19, 2014
@emberian
Copy link
Contributor Author

Note that this is only device support. Still need to support it in the renderer, I think? Possibly just need to add a simple loop in draw. Off to bed, though.

@emberian
Copy link
Contributor Author

Also, still needs to handle the case of sampler objects not being supported.

}
}

fn miplevel1(w: u16) -> u8 {
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't it be mip_level?

@kvark
Copy link
Member

kvark commented Jul 19, 2014

So how exactly this is going to work if the backend doesn't support sampler objects?
EDIT: I see your comment now about that not being supported. Technically, it's an implementation detail. Renderer may as well think there are sampler objects, but the back-end will assign the sampler state to the texture itself.

the most recently seen name")
} else {
for _ in range(0, fill) {
self.texture_info.push(unsafe { ::std::mem::zeroed() });
Copy link
Member

Choose a reason for hiding this comment

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

Unsafe for performance reasons? I'd rather just have TextureInfo::new() here, since it's clearer and not a critical hotspot.

@kvark
Copy link
Member

kvark commented Jul 19, 2014

Another concern is that TextureInfo goes a different route from ProgramMeta, which are essentially very close things by nature. The former is stored on the device side and hidden from the renderer. The latter is created by the device but then stored in the renderer.

I'm not saying one way or another is better, but I'd like us to discuss this and come up with a consistent solution (guidelines, perhaps?).

@emberian
Copy link
Contributor Author

@kvark the only reason I store the TextureInfo in the GL device is that I think I need it to figure out what target to bind the texture to to modify it later. There is no other reason, and I don't think other device implementations will need to do this.

@kvark
Copy link
Member

kvark commented Jul 19, 2014

Can we make the target a part of the handle itself then?

Regards,
Dzmitry

On Jul 19, 2014, at 14:16, Corey Richardson notifications@github.com wrote:

@kvark the only reason I store the TextureInfo in the GL device is that I think I need it to figure out what target to bind the texture to to modify it later. There is no other reason, and I don't think other device implementations will need to do this.


Reply to this email directly or view it on GitHub.

@emberian
Copy link
Contributor Author

We could, but it will double the size of the handle due to alignment. Do you see any big advantage of putting it in the handle instead of storing it inside the backend?

@emberian
Copy link
Contributor Author

(Keep in mind that only GL needs this, afaik)

/// ingrained by many years of public use of inaccurate terminology.
#[deriving(Eq, Ord, PartialEq, PartialOrd, Hash, Clone, Show)]
pub enum FilterMethod {
/// The dumbest filtering possible, nearest-neighbor interpolation.
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we could even add example images here in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The whole world would be better off if we added images in https://en.wikipedia.org/wiki/Texture_filtering instead 😈

@kvark
Copy link
Member

kvark commented Jul 19, 2014

If tartget is really the only thing out there that the device should track - then yes, I'd rather have it in the handle, which will anyway be an associated type at some point. @bjz?

On Jul 19, 2014, at 15:37, Corey Richardson notifications@github.com wrote:

We could, but it will double the size of the handle due to alignment. Do you see any big advantage of putting it in the handle instead of storing it inside the backend?


Reply to this email directly or view it on GitHub.

TextureCube,
/// A volume texture, with each 2D layer arranged contiguously.
Texture3D
// TODO: Multisampling?
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks great!

@kvark
Copy link
Member

kvark commented Jul 20, 2014

😋 thanks!

kvark added a commit that referenced this pull request Jul 20, 2014
Texture binding + creation
@kvark kvark merged commit 1906fff into gfx-rs:master Jul 20, 2014
adamnemecek pushed a commit to adamnemecek/gfx that referenced this pull request Apr 1, 2021
112: Correctness fixes from 0.2, plus a lot of goodies r=kvark a=kvark

These are changes from gfx-rs#110 back-ported to master.

Co-authored-by: Dzmitry Malyshau <dmalyshau@mozilla.com>
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.

Implement Texture creation and binding
3 participants