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

add Text Area Description to header reader/writer #64

Merged
merged 8 commits into from
Aug 15, 2023

Conversation

floriandeboissieu
Copy link
Contributor

@floriandeboissieu floriandeboissieu commented Aug 1, 2023

In the current PR, I propose an implementation of the VLR Text Area Description for header reader/writer.

In writeLAS.cpp, when the data of the text area is more than 65535 characters (2^16-1), it is truncated.

By the way, I also removed an excessive indentation in readher.cpp.

I PR on master branch as I noticed that devel branch was much older.

Copy link
Collaborator

@Jean-Romain Jean-Romain left a comment

Choose a reason for hiding this comment

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

This looks good. Please just remove nullterminate at two locations. Use Rcpp:as instead.

if (vlr.record_id == 3) // TextArea
{
CHAR* text_area_description = (CHAR*) vlr.data;
lvlr.push_back(nullterminate(text_area_description, vlr.record_length_after_header));
Copy link
Collaborator

Choose a reason for hiding this comment

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

The standard already imposes that the string are null terminated. No need to enforce it. The nullterminate function is only a trick to make the CRAN happy with (valid) non null terminated string (but we loose 1 character). We should not use it otherwise.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, sorry for the iterative commit...

src/writeLAS.cpp Outdated
CharacterVector desc = vlr["description"];
std::string sdesc = as<std::string>(desc);
I32 sascii_size = sascii.size();
if ((sizeof(CHAR)*sascii_size) > U16_MAX)
Copy link
Collaborator

@Jean-Romain Jean-Romain Aug 1, 2023

Choose a reason for hiding this comment

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

Be careful if you truncate the string it must be null-terminated. You must add \0 at position U16_MAX-1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have added a null terminator to text of any length as it is not done by LASlib.
Therefore, length_after_header == nchar(vlr$text_area_description)+1 always.

@Jean-Romain Jean-Romain merged commit b2c6804 into r-lidar:master Aug 15, 2023
5 checks passed
@Jean-Romain
Copy link
Collaborator

Thank you. Nice to see you back

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.

None yet

2 participants