[Xcb] Fwd: Changing <valueparam> to <switch> in render extension

[Christian Linhart]

Thank you for your contribution, Jaya.

I think that the diffs and Jaya's analysis show that it is quite feasible
to change <valueparam> to <switch> in existing protocol definitions.

Here's my reasoning:

1. The function xcb_render_create_picture_sizeof is missing when using <switch>.
   But it is probably a rather simple change in the generator to generate it again.
   Maybe we'll need an xml-attribute such that <switch> is generated in "valueparam compatibility mode",
   so that the additional sizeof-functions are only generated in those cases where they are needed.
   
   I think we should postpone changing the generator until all pending patches are merged into upstream
   and until we'll have done some of the planned code-cleanup stuff.
   ( To have a stable basis and to avoid too much merging. )

2. The change of type of the parameter "value_list" as mentioned in point 4 of Jaya's analyis
   is probably backward compatible due to the following reasons:

   2.1. It is calling compatible and linker compatible. 
        Reasons: 
	  - C converts all pointer types to void* implicitly.
	  - C does not include types in linktime-names of functions. ( contrary to C++ )

   2.2. There is strong evidence that the data to be passed to that parameter is the same in both vases, 
        i.e., the same memory layout, 
        because in both cases it is simply the protocol representation of this value_list.
   
   Of course this requires closer analysis and a testcase before we can be sure.

3. The rest is just new stuff added which does not cause a compatibility problem.

So, we have a good chance that we will be able to replace all <valueparam> with <switch>.
This will also mean that we will then be able to drop support for <valueparam> in the generator,
which will help to simplify the generator ( which is always a good thing ).

What do you think?

Chris

On 10/18/14 10:34, Jaya Tiwari wrote:
> 
> I am an OPW intern applicant for round 9 and interested in server-side xcb under mentorship of Christian Linhart.
> 
> For my initial small contribution,
> 
> I have changed <valueparam> to <switch> in two requests of the render extension.
> The reason is that <switch> is recommended in the xml-xcb-spec instead of <valueparam> and the <switch> contains a more precise description of the protocol.
> 
> I have following observations :
> 
> 1.xcb_render_create_picture_value_list_t :
>   A new structure with value mask names is added.
> 
> 2. Size computed by function xcb_render_create_picture_value_list_unpack
> (as used by function xcb_render_create_picture_value_list_sizeof)
> does essentially the same as the expression with popcount in xcb_render_create_picture_sizeof, but xcb_render_create_picture_
> sizeof computes the size of the whole request "render_create_picture"
> and xcb_render_create_picture_value_list_sizeof only computes the size of the switch ( with name "value_list" )
> So it is not directly compatible because the name of the function is different, so it won't compile or link with existing code which uses the previous version.
> 
> 3. parameter type const uint32_t* of value_list in xcb_render_create_picture and xcb_render_create_picture_checked changed to const void*.
> 
> 4. In xcb_render_create_picture, again instead of initializing xcb_parts with popcounting elements from value_list, it checks all possible masks using xcb_render_create_picture_value_list_sizeof and initializes xcb_parts.
> 
> 5. The work of sending request is done in xcb_render_create_picture, same things is again repeated in xcb_render_create_picture_aux (i dont really have much idea of differences in aux and non-aux functions)
> 
> 
> Along is a xml (render extension) diff along with libxcl implementation header and c files attached.
> 
> Please comment on the diffs and observations and correct me wherever my understanding fails.
> 
> Thankyou.
> 
> Regards,
> Jaya
> 
> 
> 
> 
> _______________________________________________
> Xcb mailing list
> Xcb at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/xcb