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

allow multiple attributes per style-spec property #6262

Closed
wants to merge 1 commit into from

Conversation

mollymerp
Copy link
Contributor

@mollymerp mollymerp commented Mar 1, 2018

This refactor is necessary for #2732 because *-pattern properties are cross-faded and require more data to be passed to the shaders – the x,y coordinates of the top-left and bottom-right corners of up to four icons in an image_atlas, and the pixel ratio of the atlas – that can be packed in one vec4 attribute.

I added a function that takes a property and type and returns the PaintVertexArray constructor for the layout needed for that property/type/expression type. This is where we will be able to add exceptions to the default paint vertex array layouts for properties with multiple attributes.

I haven't settled on a good way to extend the populatePaintArrays functionality to accommodate special cases yet, but it might make sense to work that out in the follow up PR implementing line-pattern property functions.

Launch Checklist

  • briefly describe the changes in this PR
  • write tests for all new functionality
  • document any changes to public APIs
  • post benchmark scores
  • manually test the debug page

this.type = type;
this.statistics = { max: -Infinity };
}

defines() {
return [`#define HAS_UNIFORM_u_${this.name}`];
return this.names.map((name) => { return `#define HAS_UNIFORM_u_${name}`; });
Copy link
Member

Choose a reason for hiding this comment

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

Nit: you can omit braces and return here.

} else {
gl.uniform1f(program.uniforms[`u_${name}`], value);
}
});
Copy link
Member

Choose a reason for hiding this comment

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

Nit: let's avoid using forEach and closures in favor of a for .. of loop.

components: type === 'color' ? 2 : 1,
offset: 0
};
});
Copy link
Member

Choose a reason for hiding this comment

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

Nit: you can omit braces and return here too if you surround the object with (...).

@mollymerp
Copy link
Contributor Author

folding this into #6289 – I incorporated your requested changes @mourner 😸

@mollymerp mollymerp closed this Mar 7, 2018
@jfirebaugh jfirebaugh deleted the multi-attribute-properties branch April 29, 2018 23:41
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