GNU bug report logs - #73384
[PATCH] Draw coloured stipples on NS

Previous Next

Full log

View this message in rfc822 format

From: Alan Third <alan <at> idiocy.org>
To: Arash Esbati <arash <at> gnu.org>
Cc: Rudolf Adamkovič <rudolf <at> adamkovic.org>, 73384 <at> debbugs.gnu.org, Stefan Kangas <stefankangas <at> gmail.com>, JD Smith <jdtsmith <at> gmail.com>, Ben Simms <bsimms.simms <at> gmail.com>
Subject: bug#73384: [PATCH] Draw coloured stipples on NS
Date: Wed, 18 Dec 2024 21:55:44 +0000

> +#ifdef NS_IMPL_COCOA
> +/* Returns a cached CGImageMask of the stipple pattern */
> +- (CGImageRef)stippleMask
> +{
> +  if (stippleMask == nil) {
> +    CGDataProviderRef provider = CGDataProviderCreateWithData (NULL, [bmRep bitmapData],
> +                                                             [self sizeInBytes], NULL);
> +    CGImageRef mask = CGImageMaskCreate(
> +                                        [self size].width,
> +                                        [self size].height,
> +                                        8, 8, [self size].width,
> +                                        provider, NULL, 0);

There's some weird formatting in this patch. Some of it looks like
it's perhaps due to email, but other bits, like the above, just look
wrong.

Other things I've noticed include C++ comments, //, instead of C
comments, /* */. Large blocks of code with no whitespace that is a bit
hard to follow. It would be nicer if it was broken up into logical
blocks.


> +    r = NSMakeRect (s->x, s->y + box_line_width,
> +        s->background_width,
> +        s->height - 2 * box_line_width);
<snip>
> +    NSRectFill (r);
> +    s->background_filled_p = 1;
> +    CGImageRef mask = [dpyinfo->bitmaps[face->stipple - 1].img stippleMask];
> +    CGRect bounds = CGRectMake (s->x, s->y + box_line_width,
> +        s->background_width,
> +        s->height - 2 * box_line_width);

NSRect and CGRect are the same thing, so here "r" and "bounds" are
identical. It might be worth just having one variable.

> +	  else if (s->stippled_p) {

Opening braces go on new lines.

Really that's it, Just some polishing required and a proper commit
message. Otherwise it looks OK to me.

I take it this doesn't require the addition of any extra build flags
to bring in CoreGraphics?
-- 
Alan Third

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.