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

DXF export: fix crash when opening dialog #41900

Closed
wants to merge 1 commit into from

Conversation

rouault
Copy link
Contributor

@rouault rouault commented Feb 27, 2021

This is due to the changes done in #39577
in which QgsLayerTreeView no longer uses the model set with setModel(),
but wraps it in a QgsLayerTreeProxyModel.
QgsDxfExportDialog expected QgsLayerTreeView::model() to return the model
it provided.

Note: the fix is probably incomplete. I noticed that when using
Select all/Unselect all, the view isn't automatically refreshed. One must
trigger a redraw (like resizing the dialog) to see the effect.

@nyalldawson @elpaso You might want to follow-up for the remaining issue I mention above

This is due to the changes done in qgis#39577
in which QgsLayerTreeView no longer uses the model set with setModel(),
but wraps it in a QgsLayerTreeProxyModel.
QgsDxfExportDialog expected QgsLayerTreeView::model() to return the model
it provided.

Note: the fix is probably incomplete. I noticed that when using
Select all/Unselect all, the view isn't automatically refreshed. One must
trigger a redraw (like resizing the dialog) to see the effect.
@rouault rouault added backport release-3_18 Bug Either a bug report, or a bug fix. Let's hope for the latter! labels Feb 27, 2021
@github-actions github-actions bot added this to the 3.20.0 milestone Feb 28, 2021
@elpaso elpaso self-requested a review February 28, 2021 08:26
elpaso added a commit to elpaso/QGIS that referenced this pull request Feb 28, 2021
Folloup qgis#41900

Also hides aspatial layers from export candidates. I'm not sure
if this is correct but thre was an assert that was hit
when aspatial layers were present, now I don't know if the
assert (which is about the size of the layers list and
the layer list from custom layer order, that only contains
spatial layers) has been there forever and no one actually
tested in dev mode with aspatial layers.

In any event, getting aspatial layers back into the list
is easy but then we'd need to alter the logic that builds
the ordered layers list and remove the assert.
@elpaso elpaso mentioned this pull request Feb 28, 2021
@elpaso
Copy link
Contributor

elpaso commented Feb 28, 2021

@rouault see: #41918 for a simpler approach.

@rouault rouault closed this Feb 28, 2021
nyalldawson pushed a commit that referenced this pull request Feb 28, 2021
Folloup #41900

Also hides aspatial layers from export candidates. I'm not sure
if this is correct but thre was an assert that was hit
when aspatial layers were present, now I don't know if the
assert (which is about the size of the layers list and
the layer list from custom layer order, that only contains
spatial layers) has been there forever and no one actually
tested in dev mode with aspatial layers.

In any event, getting aspatial layers back into the list
is easy but then we'd need to alter the logic that builds
the ordered layers list and remove the assert.
github-actions bot pushed a commit that referenced this pull request Feb 28, 2021
Folloup #41900

Also hides aspatial layers from export candidates. I'm not sure
if this is correct but thre was an assert that was hit
when aspatial layers were present, now I don't know if the
assert (which is about the size of the layers list and
the layer list from custom layer order, that only contains
spatial layers) has been there forever and no one actually
tested in dev mode with aspatial layers.

In any event, getting aspatial layers back into the list
is easy but then we'd need to alter the logic that builds
the ordered layers list and remove the assert.
nyalldawson pushed a commit that referenced this pull request Feb 28, 2021
Folloup #41900

Also hides aspatial layers from export candidates. I'm not sure
if this is correct but thre was an assert that was hit
when aspatial layers were present, now I don't know if the
assert (which is about the size of the layers list and
the layer list from custom layer order, that only contains
spatial layers) has been there forever and no one actually
tested in dev mode with aspatial layers.

In any event, getting aspatial layers back into the list
is easy but then we'd need to alter the logic that builds
the ordered layers list and remove the assert.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Either a bug report, or a bug fix. Let's hope for the latter!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants