[m-rev.] for review: more bits and pieces for the opengl binding

[Fergus Henderson]

On 12-Jan-2004, Julien Fischer <juliensf at students.cs.mu.OZ.AU> wrote:
> 
> extras/graphics/mercury_opengl/mogl.m:
> 	Add bindings for glRect(), glHint() and glIsEnabled().

That looks fine.

Just one comment:

> Index: mogl.m
...
> +is_enabled(Flag, IsEnabled, !IO) :-
> +	( Flag = clip_plane(I) ->
> +	  	is_enabled_3(control_flag_to_int(Flag), I, IsEnabled, !IO)
> +	; Flag = light(I) ->
> +	  	is_enabled_3(control_flag_to_int(Flag), I, IsEnabled, !IO)
> +	;
> +		is_enabled_2(control_flag_to_int(Flag), IsEnabled, !IO)
> +	).
> +
> +:- pred is_enabled_2(int::in, bool::out, io::di, io::uo) is det.
> +:- pragma foreign_proc("C",
> +	is_enabled_2(I::in, R::out, IO0::di, IO::uo),
> +	[may_call_mercury, promise_pure],
> +"
> +	if(glIsEnabled(control_flag_flags[I]))
> +		R = ML_bool_return_yes();
> +	else
> +		R = ML_bool_return_no();
> +	IO = IO0;
> +").
> +
> +:- pred is_enabled_3(int::in, int::in, bool::out, io::di, io::uo) is det.
> +:- pragma foreign_proc("C",
> +	is_enabled_3(I::in, J::in, R::out, IO0::di, IO::uo),
> +	[may_call_mercury, promise_pure],
> +"
> +	if(glIsEnabled(control_flag_flags[I] + J))
> +		R = ML_bool_return_yes();
> +	else
> +		R = ML_bool_return_no();
> +	IO = IO0;
> +").

The code duplication there is undesirable.  Also, the logic is vulnerable
to breakage if future revisions of GL introduce new control flags.
Likewise for a couple of similar places in the existing code in mogl.m
(enable/3 and disable/3).

It would be nicer to do

	is_enabled(Flag, IsEnabled, !IO) :-
		control_flag_to_int_and_offset(Flag, Int, Offset),
		is_enabled_2(Int, Offset, !IO).

or

	is_enabled(Flag, IsEnabled, !IO) :-
		is_enabled_2(control_flag_to_int(Flag),
			control_flag_to_offset(Flag), !IO).

where the old is_enabled_2 is deleted and the old is_enabled_3 is renamed
is_enabled_2.

However, that would be better done as a separate change.

-- 
Fergus Henderson <fjh at cs.mu.oz.au>  |  "I have always known that the pursuit
The University of Melbourne         |  of excellence is a lethal habit"
WWW: <http://www.cs.mu.oz.au/~fjh>  |     -- the last words of T. S. Garp.
--------------------------------------------------------------------------
mercury-reviews mailing list
post:  mercury-reviews at cs.mu.oz.au
administrative address: owner-mercury-reviews at cs.mu.oz.au
unsubscribe: Address: mercury-reviews-request at cs.mu.oz.au Message: unsubscribe
subscribe:   Address: mercury-reviews-request at cs.mu.oz.au Message: subscribe
--------------------------------------------------------------------------