Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

r.flowaccumulation: Handle null values properly #957

Merged
merged 1 commit into from
Oct 17, 2023

Conversation

HuidaeCho
Copy link
Member

No description provided.

@HuidaeCho HuidaeCho merged commit 59a9207 into OSGeo:grass8 Oct 17, 2023
7 checks passed
@HuidaeCho HuidaeCho deleted the rflowaccumulation_handle_null branch October 17, 2023 19:23
@HuidaeCho HuidaeCho changed the title r.flowaccumulate: Handle null values properly r.flowaccumulation: Handle null values properly Oct 17, 2023
Comment on lines +35 to +41
#ifdef _MAIN_C_
#define GLOBAL
#else
#define GLOBAL extern
#endif

GLOBAL CELL cell_null;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using Rast_is_c_null_value would avoid the need for an award global variable. Would Rast_is_c_null_value work here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, Rast_is_c_null_value should also work. That global was my attempt for micro-optimization. Just did a benchmark test against Rast_is_c_null_value. Well.. not much difference... Maybe, GCC can optimize out this function call as inline. Yeah.. not worth it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Benchmark! Nice! The function is in fact a macro, so it should be always inlined.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah! I missed #ifndef Rast_is_c_null_value!

#ifndef Rast_is_c_null_value
int Rast_is_c_null_value(const CELL *cellVal)
{
    /* Check if the CELL value matches the null pattern */
    return *cellVal == (CELL)0x80000000;
}
#endif

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants