Contents 1 Intial feedback for wip/point-sprites 1.1 cogl-material: Add a property for setting the point size 1.2 cogl-material: Add support for point sprites 1.3 Other points to consider

Intial feedback for wip/point-sprites

cogl-material: Add a property for setting the point size

In cogl-context.h point_size_cache has been inserted in between items of state relating to depth at a different indentation and looks a bit odd. (possibly just a side effect from a rebase)

Yeah, I don't know what happened there. I'll move it down. This is why I hate putting structs in columns! --Bpeel 14:30, 30 June 2010 (UTC)

in CoglMaterialBigState I think I'd prefer that point_size be a float instead of GLfloat. If you want to you can cast it when you actually call GL, though personally I consider float and GLfloat equivalent. Conceptually I try to consider Cogl's state tracking to be independent of OpenGL and although we don't have a driver abstraction layer I think GL should only be required when flushing state.

Ok, I agree. I'm completely fine with assuming GLfloat is the same as float. --Bpeel 14:30, 30 June 2010 (UTC)

For cogl_material_set_point_size you have a comment like:

"Note that not all point sizes will be supported but if the chosen size isn't available then the nearest size will be used."

This is a bit confusing. "nearest" to what?

I could change it to something like "Note that typically the GPU will only support a limited minimum and maximum range of point sizes. If the chosen point size is outside that range then the nearest value within that range will be used instead". --Bpeel 14:30, 30 June 2010 (UTC)

Yep that seems better to me. --RobertBragg 14:39, 1 July 2010 (UTC)

Also although we don't really have a precedent for being able to query GPU/driver constants in Cogl we could consider adding a cogl_renderer_ API for this and also make the function return a GError (which we do have a precedent for)

I'm not sure about throwing a GError if it fails. When an error is thrown I would expect the call to have no effect but I think in this case just choosing the nearest value is quite useful. I think it would be wrong throw an error and still choose the nearest value. I think it would be reasonable to just have the API to query the range. --Bpeel 14:30, 30 June 2010 (UTC)

I keep changing my mind over this, but currently I'm in agreement that we don't need the GError here. In general I'm not really a big fan of just saying "you must query such and such to find out if this will work", because I think in reality developers are lazy and don't always reference documentation (e.g. they often copy and paste code, or read the docs once and forget the details later). So sometimes I think an explicit GError makes it easier for the developer to identify - oh this might not always work maybe I can add a fallback. --RobertBragg 14:39, 1 July 2010 (UTC)

The cogl_renderer_ API as in the CoglDesign wiki page all takes a #CoglRenderer argument (as opposed to the current Cogl context APIs where it's implied) so we'd need to have:

• A new stub CoglRenderer object.
• A way to get the renderer from the current context:

renderer = cogl_get_renderer ();

• And then an API to query the constant:

cogl_renderer_get_driver_constant (COGL_DRIVER_CONSTANT_XYZ);

I think the renderer stuff can be done as a separate piece of work though if we at least add a mechanism for the user to get feedback when the size isn't supported.

Even if we add a GError status to cogl_material_set_point_size, if the user passes NULL and ignores the error what will the semantics be and then what will the semantics of cogl_material_get_point_size() be? E.g. if Cogl silently clamps it to the largest supported size will get_point_size() return the users original size or the real clamped size?

Good question. I think this is extra tricky when you consider shaders. I would expect that we would expose the point size as a vertex attribute and in that case it's probably fine to have any value in the attribute. Presumably it would only be the final value that gets written to gl_PointSize from the vertex shader that would end up being clamped. I think it would be best if cogl_material_set_point_size() just always succeeds and then if an app needs to worry about it failing it can query the point size range itself. --Bpeel 14:30, 30 June 2010 (UTC)

Sounds sensible to me. --RobertBragg 14:39, 1 July 2010 (UTC)

cogl-material: Add support for point sprites

cogl_material_set_layer_point_sprite_coords: I wonder if this should also return a GError.

Yeah, that sounds good --Bpeel 14:30, 30 June 2010 (UTC)

I know it's verbose but I wonder if this would be a better name: cogl_material_set_layer_point_sprite_coords_enabled()

Yeah, the existing name isn't very good. If we can't think of anything shorter than your name sounds better than mine. --Bpeel 14:30, 30 June 2010 (UTC)

cogl_material_get_layer_point_sprite_coords

assuming we add a GError arg to the _set we'll need to define the semantics for when the user passes NULL and whether this _get should return the users original value or the real value according to what the driver supports. Value undefined might be the right definition, not sure?

I think it should return FALSE if set_layer_point_sprite_coords_enabled() fails. I think if an error is thrown the function should try its best to not have any affect. I think passing NULL as the GError means you don't care if there was an error not to try and do something to work around the error. So I think the function should always do the same thing whether or not error==NULL. --Bpeel 14:30, 30 June 2010 (UTC)

Ok, sounds good. --RobertBragg 14:39, 1 July 2010 (UTC)

Other points to consider

We probably want to expose a vertex attribute to set the point size. I think this doesn't particularly impede the current patches because the usage pattern would then match cogl_material_set_color(). So there is a default attribute value for all of the vertex attributes attached to the material but then the value would get overriden if a VBO is used that defines the attribute. However this is currently painful to implement because GL < 2.0 does not support per-point sizes. The per-point size is only exposed through a GLSL vertex output variable so it will be hard to implement without a Cogl GLSL backend. I don't know whether ARBvp supports this but that would be good to look into. The current shader backends only affect the fragment program so that doesn't map well to implementing the point parameters.

We could also expose the point parameter settings from GL 1.x. These let you for example attenuate the point size based on the z value.