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

Defer inlining of blocks until actually required (trac ticket #7106) #251

Closed
wants to merge 2 commits into from

Conversation

atlight
Copy link
Contributor

@atlight atlight commented Oct 23, 2017

they are part of.
attributes, they will also have a Block attribute indicating what block
they are part of. (Note, in GDAL 2.2.2 and earlier this attribute was called
BlockName.)
Copy link
Member

Choose a reason for hiding this comment

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

"GDAL < 2.3"
(there will likely be a GDAL 2.2.3 without those trunk-only changes)

<li> The entities in the blocks layer should have the BlockName field
populated.
<li> The entities in the blocks layer should have the Block field
populated. (Note, in GDAL 2.2.2 and earlier this attribute was called
Copy link
Member

Choose a reason for hiding this comment

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

"GDAL < 2.3"

/* Convert to polygon, multipolygon, multilinestring or multipoint */
/* -------------------------------------------------------------------- */
// Restore old inline blocks setting
bInlineBlocks = bOldInlineBlocks;
Copy link
Member

Choose a reason for hiding this comment

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

Missing restoration in above early return in error code path

/* point. */
/************************************************************************/
OGRDXFFeature *OGRDXFLayer::InsertBlockReference( const CPLString& osBlockName,
OGRDXFInsertTransformer oTransformer, OGRDXFFeature* const poFeature )
Copy link
Member

Choose a reason for hiding this comment

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

"const OGRDXFInsertTranformer&" to save a copy

@@ -74,77 +73,73 @@ void OGRDXFBlocksLayer::ResetReading()

{
iNextFID = 0;
iNextSubFeature = 0;
while (!apoPendingFeatures.empty())
Copy link
Member

Choose a reason for hiding this comment

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

I guess the pop'ed feature should be deleted

Copy link
Member

Choose a reason for hiding this comment

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

And probably that the destructor of the layer should also do that cleanup

@atlight
Copy link
Contributor Author

atlight commented Oct 23, 2017

I'm not too familiar with GitHub pull requests so let me know if I am doing something in a non-preferred way.

@rouault
Copy link
Member

rouault commented Oct 23, 2017

Hum for some reason Travis-CI checks have not been run.

@rouault
Copy link
Member

rouault commented Oct 24, 2017

I've pushed your branch to my repo, and with a small fix rouault@4e7c055 to please cppcheck, all tests are green with Travis-CI: https://travis-ci.org/rouault/gdal2/builds/292020340
@atlight So I suppose I can commit that work ?

@atlight
Copy link
Contributor Author

atlight commented Oct 24, 2017

Yes. Thank you as always!

@rouault
Copy link
Member

rouault commented Oct 24, 2017

Committed in r40548

@rouault rouault closed this Oct 24, 2017
rouault added a commit that referenced this pull request Oct 24, 2017
kwrobot pushed a commit to aashish24/gdal-svn that referenced this pull request Oct 24, 2017
@atlight atlight deleted the t-7106 branch May 18, 2023 02:47
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