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

AttachmentOptions missed logic #168

Open
kdziamura opened this issue May 26, 2020 · 3 comments
Open

AttachmentOptions missed logic #168

kdziamura opened this issue May 26, 2020 · 3 comments

Comments

@kdziamura
Copy link
Contributor

kdziamura commented May 26, 2020

During the investigation, I found that parameter AttachmentOptions['attach'] is never used, but fully documented and has typescript declaration:

* @property {number} [attach] The attachment point. Defaults

attach?: number;

An actual attachment point is being selected here:

let attachmentPoint = getAttachmentPointForFormat(format);

I'll be glad to create a pull request, but I confused with the idea of this parameter. Is it redundant legacy code and should be removed, or is it an option, that should be used as an attachment point if defined?

@munrocket
Copy link
Contributor

Yeah, looks like an outdated comment that become a property in typescript.

Here usage of attachments

{ attachment: tex, target: gl.TEXTURE_3D, layer: 0, },

@kdziamura
Copy link
Contributor Author

kdziamura commented May 27, 2020

So the order of attachments is important. Maybe it's better to have the option of using a specific attachment point? Or, at least, add attachment point into output object? It would be great to have the availability of reusing it in some cases. To use with gl.drawBuffers for example. Here is the manual definition of them, and it could be unclear at first glance:


I mean that current logic isn’t transparent. The other option is to add some parameter/s, which will run gl.drawBuffers under the hood, but it’s unclear and not flexible solution as well

@greggman
Copy link
Owner

Good catch. I'm not sure off top of my head what the best solution is. twgl is not trying to cover all of WebGL. It's just trying to make some things less verbose.

I don't think generating an input for drawBuffers is real all that useful. It's a really tiny API. Just an array with 4-8 constants. Does that really need an helper?

The simplest thing would be to remove it. Clearly no one is using it if it never worked. .

Otherwise the intent was

let attachmentPoint = attachmentOptions.attach || getAttachmentPointForFormat(format);

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

3 participants