[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#73384: [PATCH] Draw coloured stipples on NS
From: |
Alan Third |
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