Clutter Wiki

Views
From ClutterProject
Jump to: navigation, search

This design is now implemented and merged into master

Design

We will have a new CoglHandle type to represent a path. The ctx->path_nodes array will be replaced with a reference to the current path. All of the path functions will manipulate the current path handle. There will be two new API calls to change the path handle:

CoglHandle cogl_path_get (void);
void cogl_path_set (CoglHandle path);

Getting the path returns a handle to the existing path, not a copy. So if you call more functions to add to Cogl's path it will be reflected in the handle you receive. A reference will not be taken so you would need to take your own reference to keep it.

cogl_path_set will take a reference on the new path and unref the old path.

cogl_path_new is an already existing function so we can't really change it to just return a handle. Instead it will create a path and immediately set it as the current path. It will still return nothing.

A clutter actor that renders an ellipse might do something like this:

paint ()
{
  if (priv->path == COGL_INVALID_HANDLE)
    {
       cogl_path_new ();
       cogl_path_arc (0, 0, 100, 100, 0, 2 * G_PI);
       priv->path = cogl_handle_ref (cogl_path_get ());
    }

  cogl_path_set (priv->path);
  cogl_path_fill ();
}

Additionally there is a function for copying paths called cogl_path_copy(). This has copy-on-write semantics so it is relatively cheap.

Paths in the render list

When we eventually get render list support we want to be able to store a reference to the path in the list. However we still need to support the case where someone does something like this:

cogl_path_move_to (10, 10);
cogl_path_line_to (20, 20);
cogl_path_stroke_preserve ();
cogl_path_line_to (30, 30);
cogl_path_stroke ();

So in this case we can't just naïvely store a reference to the path each time it is stroked because the path is later being modified. To avoid having to implement copy-on-write I think we could store a reference to the path and a count of the nodes. The only way to modify a path currently is to append to it so it's not possible that two different primitives in the render list could use the same path reference with different nodes. So instead a path entry in the render list would be something like 'draw this path up to node 3'.

Comment
Implementing a cogl_path_copy() API with copy-on-write sounds pretty straightforward though I think. We can just use the same trick you describe above internally, and add a copy_on_write hook into the functions that can modify a path?
I.e.:
  1. Add a parent member and a parent_node_count to the CoglPath structure so copying a path with no modifications is just a case of initializing those members.
  2. Implement a copy_on_write function called by all path API that modifies a path and if it sees the parent member set it copies the parent nodes into itself and unsets the parent pointer/node_count. Looking at the clip-stack code which has to make copies of paths the actual copy looks like it's just a memcpy of path_nodes->data, and a copy of the path_nodes_min, path_nodes_max and path_nodes_size members.
This way we don't need to change how we log into the renderlist if we ever allow editing of paths.
--RobertBragg 12:24, 8 April 2010 (UTC)
Comment
I've done what you said and implemented cogl_path_copy. However it's no longer completely shallow because the clone needs to keep its own copies of the bounding box as well as the size (because both may be changed in the parent). It does however avoid copying the nodes array so it's still reasonably cheap.
--Bpeel 17:08, 8 April 2010 (UTC)

Discussion

bob the handle path thing is probably a 5 minute job though, we just need to commit to it :-)
neil yeah
neil there's some tricky api considerations there though
neil i think if we were to do the path api again then we'd have all the cogl_path_* functions directly take a path handle
neil otherwise we're going to have something like 'cogl_select_path' etc
neil or cogl_make_path_current
bob I was guessing we'd could just add something like cogl_get/set_path to get a handle for the current path
neil so if you do cogl_move_to does it conceptually make a new path or does it edit the handle you set ?
bob I gues ctx->path_nodes could be encapsulated in a CoglPath ctx->current_path and I think I would imagine cogl_move_to to create a new path if ctx->current_path = COGL_INVALID_HANDLE or modify the current path if one already exists. I would sort of expect cogl_path_stroke to unref the current path and then create a new one when it calls cogl_path_new.
bob actually maybe the case where ctx->current_path == COGL_INVALID_HANDLE should never happen if stroke always creates a new path
neil yeah that makes sense
bob if you set a path, that would unref the current and take a new reference on the set path
bob (or the other way around I guess)
neil so you don't think it would be worth making a new API if we can think of another namespace ?
neil and then saying cogl_path_* is deprecated and make them just a thin wrapper around cogl_newpath_* (cogl_get_default_path(),...)
neil i guess that's kind of awkward because if you just want to draw a simple rectangle you have to make a local variable to hold the path
bob I still cant think of concrete ways I would make a better path API, so patching up the existing one for now seems reasonable. (making a new API that takes an explicit path handle might be good but it doesn't seem worth making a new api for that)
neil ok, agreed
neil so... cogl_set_path is more like cogl_make_path_current ?
neil otherwise you might expect to be able to take a snapshot of the path in the middle of creating the path with cogl_path_get
bob every now and then I have doubts about the fact that we support fully projected paths and the semantics are that they should interact with depth just like any other gometry, but that might even be a good thing.
bob right cogl_set_path is like cogl_make_path_current, cogl_get_path could kinda let you get a snapshot of a path in its current state if you were to set a new path afterwards, not quite sure what you mean by snapshot?
bob I wouldn't expect it to make a deep copy of the current path and return you handle for that; it would directly return the current path handle.
neil yeah, i'm just nitpicking - but just by the name it would seem to me that it would make a deep copy
neil but actually who cares :)
bob right; just wondering if we have set a precedent yet in the API, we don't have have a cogl_get_source, and I think cogl_get_framebuffer is internal only api.
bob cogl_path_new is going to be a bit awkward/inconsistent
neil heh, good point
neil well, I guess it's not too bad, because you can't do anything with a path unless you make it current
neil so there'd be no point in having to do cogl_path_set(cogl_path_new())
bob yeah, should be ok
neil and otherwise we'd kind of want floating references on CoglPaths
neil but if we immediately select it then that doesn't matter
bob would it be evil to make cogl_path_new also return a handle though?
neil heh
neil that *may* be an ABI break on windows
neil it encodes loads of extra crap in the symbol name
neil or that might just be the number of arguments so it probably isn't an issue
bob if it did you might be more likely to assume you have to also call cogl_set_path which could make it misleading
neil yup
bob it does seem a shame that it's inconsistent, even though it would be more verbose to use if it were consistent
neil true
neil we can't really do anything about it though unless we call the path handle something else
neil we could add a cogl_retained_path_new maybe ?
neil that's pretty ugly though
bob yeah, can't really think of a nice way to have it.
Personal tools