Skip to content

The byte image change was only meant to cover 1-band images#7871

Merged
PaulWessel merged 2 commits intomasterfrom
three-vs-one
Oct 5, 2023
Merged

The byte image change was only meant to cover 1-band images#7871
PaulWessel merged 2 commits intomasterfrom
three-vs-one

Conversation

@PaulWessel
Copy link
Member

Need to ensure that the image has only one band and no indexed color. Closes #7827.

Need to ensure that the image has only one band and no indexed color.  Closes #7827.
@PaulWessel PaulWessel added the bug Something isn't working label Oct 5, 2023
@PaulWessel PaulWessel requested a review from seisman October 5, 2023 08:58
@PaulWessel PaulWessel self-assigned this Oct 5, 2023
@joa-quim
Copy link
Member

joa-quim commented Oct 5, 2023

Wait, but now that I think on it, the error condition is a regression in itself. We used to allow indexed images without color maps and in that case it was attributed an automatic 0-255 one.

@PaulWessel
Copy link
Member Author

Well, a 1-byte image without color index should work unless you give -C. no?

@PaulWessel
Copy link
Member Author

Perhaps need more checks. We have this:

	byte_image_no_cmap = (I && I->type == GMT_UCHAR && I->n_indexed_colors == 0);
	if (byte_image_no_cmap && !Ctrl->C.active) {
		GMT_Report (API, GMT_MSG_ERROR, "Byte image without indexed color requires a CPT via -C\n");
		Return (GMT_RUNTIME_ERROR);
	}

But perhaps if no -C then byte_image_no_cmap should be false.

@PaulWessel
Copy link
Member Author

Check this PR - works for ex52 and allows gray image. @joa-quim please check.

@PaulWessel PaulWessel merged commit c6c1fe4 into master Oct 5, 2023
@PaulWessel PaulWessel deleted the three-vs-one branch October 5, 2023 09:40
@joa-quim
Copy link
Member

joa-quim commented Oct 5, 2023

I have not yet pulled this merged PR and see that this errors

gmt grdimage rgb_image.tiff+b0 -png lixo
grdimage [ERROR]: Byte image without indexed color requires a CPT via -C

but shouldn't fail. It used to work because in those cases we provide a default color map. That is, this error check all together must be removed. Otherwise we are breaking previous behavior. The change was to allow -C, not to make it mandatory.

@PaulWessel
Copy link
Member Author

Sure, that is why I added the check on -C. if no -C then should not get into that branch at all and instead paint gray. Please test it

@PaulWessel
Copy link
Member Author

master has

byte_image_no_cmap = (I && I->type == GMT_UCHAR && I->n_indexed_colors == 0 && I->header->n_bands == 1 && Ctrl->C.active);

So that is only true if 1 band, no index map, and -C is set. Your example has no -C so not updated.

@joa-quim
Copy link
Member

joa-quim commented Oct 5, 2023

OK, but can't update right now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants