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

Mask renderer #1380

Merged
merged 9 commits into from May 24, 2014

Conversation

Projects
None yet
5 participants
@mhugo
Contributor

mhugo commented May 21, 2014

Replacement of the issue #1254

This adds a new 'mask' feature renderer that allows to fill the exterior part of polygons. Basically a mask renderer embeds another feature renderer. All the features to render are collected, a big "inverted" polygon is built and this geometry is rendered using the embedded renderer.
The "inverted" polygon is built without union or difference operations that are too slow and unneeded for an operation of rendering: the final polygon is just an aggregation of rings, where exterior rings become interior rings and so on, without any validity check.
Embedded renderers that have differents symbols or more than one symbol per feature are useable. In this case, one inverted polygon will be built for each category of symbols. It is up to the user to play with transparency to see inverted polygons below some others.

This is a first request to see if everyone agree here. So SIP bindings and unit tests are still missing. The plan is to merge everything before the feature freeze.
@wonder-sk @nyalldawson do you think could you have a look ?

The only "strange" behaviour (but not critical) I've noticed so far is with the highlight: when using the identification tool, you have to click on the feature (interior), but its exterior (and not the interior) will be highlighted. I don't see any obvious way to change this (any idea ?). Vertex markers and selection are renderer correctly (within the interior)

@mhugo mhugo referenced this pull request May 21, 2014

Closed

Add exterior fill mode #1254

@mhugo

This comment has been minimized.

Show comment
Hide comment
@mhugo

mhugo May 21, 2014

Contributor

assigned to @wonder-sk (like for the #1254)

Contributor

mhugo commented May 21, 2014

assigned to @wonder-sk (like for the #1254)

@nyalldawson

This comment has been minimized.

Show comment
Hide comment
@nyalldawson

nyalldawson May 22, 2014

Contributor

@mhugo - this request is missing a ui file

Contributor

nyalldawson commented May 22, 2014

@mhugo - this request is missing a ui file

@mhugo

This comment has been minimized.

Show comment
Hide comment
@mhugo

mhugo May 22, 2014

Contributor

Oups ! Added

Contributor

mhugo commented May 22, 2014

Oups ! Added

@wonder-sk

This comment has been minimized.

Show comment
Hide comment
@wonder-sk

wonder-sk May 22, 2014

Member

Hi Hugo

I haven't tried to run it yet, but from the first look the code looks good to me! Please go ahead and finalize the PR.

Few minor notes:

  • clone() should probably also clone the sub renderer
  • probably we could store just geometries instead of the whole features - or even better directly build the final geometries without the temporary list of features?
  • maybe instead of QByteArray we could use just a simple value type and in case of more symbols a hash made from pointers?
Member

wonder-sk commented May 22, 2014

Hi Hugo

I haven't tried to run it yet, but from the first look the code looks good to me! Please go ahead and finalize the PR.

Few minor notes:

  • clone() should probably also clone the sub renderer
  • probably we could store just geometries instead of the whole features - or even better directly build the final geometries without the temporary list of features?
  • maybe instead of QByteArray we could use just a simple value type and in case of more symbols a hash made from pointers?
@nyalldawson

This comment has been minimized.

Show comment
Hide comment
@nyalldawson

nyalldawson May 22, 2014

Contributor

@mhugo is there any way for a renderer to know whether its being used as a mask? Reason I ask is that shapeburst needs to handle things a bit differently in a mask mode, so that the burst isn't also shading from the edge of the canvas.

Otherwise, I gave this a test and I'm a bit torn. On one hand it works well for what it is, and its really nice to be able to use any of qgis' symbol styles in a mask. On the other hand, I'm strongly against the need to have a layer multiple times in a project for solely cartographic reasons. The original implementation allowed exterior fills to be stacked in a single layer with non exterior fills, which is no longer possible in this version. That's a major downside for me. So, from a purely cartographic point of view, there's positives and negatives to both approaches...

Contributor

nyalldawson commented May 22, 2014

@mhugo is there any way for a renderer to know whether its being used as a mask? Reason I ask is that shapeburst needs to handle things a bit differently in a mask mode, so that the burst isn't also shading from the edge of the canvas.

Otherwise, I gave this a test and I'm a bit torn. On one hand it works well for what it is, and its really nice to be able to use any of qgis' symbol styles in a mask. On the other hand, I'm strongly against the need to have a layer multiple times in a project for solely cartographic reasons. The original implementation allowed exterior fills to be stacked in a single layer with non exterior fills, which is no longer possible in this version. That's a major downside for me. So, from a purely cartographic point of view, there's positives and negatives to both approaches...

@mhugent

This comment has been minimized.

Show comment
Hide comment
@mhugent

mhugent May 23, 2014

Contributor

assigned to @nyalldawson

Contributor

mhugent commented May 23, 2014

assigned to @nyalldawson

@mhugo

This comment has been minimized.

Show comment
Hide comment
@mhugo

mhugo May 23, 2014

Contributor

Thanks for your reviews.
@wonder-sk

  • about the clone() : exact, I forgot that, thanks
  • exact, there is no need to store all the features, the combined geometry can be built during renderFeature()
  • considering the small number of categories, I am not sure we will gain something with a hash map

@nyalldawson In order to know in a renderer if it will be used as a mask, the only solution I see is to flag the QgsGeometry in a way or another, saying that it spans the entire plane (and pass this flag to renderPolygon for instance). Maybe something to think about when refactoring the geometry class (@mhugent?)

I know this is a bit more limited than my first proposal. But, in order to have something both powerful (allowing to mix inverted and regular polygons) and clean, it will apparently need some important work / refactoring than I am not currently available for or able to do.

I am going to fix the remaining issues and complete with SIP and unit tests.
One side note: I am not sure "mask renderer" is the best name we could find. Actually, I am now thinking of something like "inverted polygon renderer" or "inside out renderer" both for the class name and the name displayed to the user. Any comment ?

Contributor

mhugo commented May 23, 2014

Thanks for your reviews.
@wonder-sk

  • about the clone() : exact, I forgot that, thanks
  • exact, there is no need to store all the features, the combined geometry can be built during renderFeature()
  • considering the small number of categories, I am not sure we will gain something with a hash map

@nyalldawson In order to know in a renderer if it will be used as a mask, the only solution I see is to flag the QgsGeometry in a way or another, saying that it spans the entire plane (and pass this flag to renderPolygon for instance). Maybe something to think about when refactoring the geometry class (@mhugent?)

I know this is a bit more limited than my first proposal. But, in order to have something both powerful (allowing to mix inverted and regular polygons) and clean, it will apparently need some important work / refactoring than I am not currently available for or able to do.

I am going to fix the remaining issues and complete with SIP and unit tests.
One side note: I am not sure "mask renderer" is the best name we could find. Actually, I am now thinking of something like "inverted polygon renderer" or "inside out renderer" both for the class name and the name displayed to the user. Any comment ?

@nyalldawson

This comment has been minimized.

Show comment
Hide comment
@nyalldawson

nyalldawson May 23, 2014

Contributor

@mhugo - hmmm... if there's no way fill renderers can be notified that they are being used as a mask, then it effectively writes off the possibility of exterior shapeburst fills, which is a real shame! @wonder-sk do you have any ideas how this could be achieved? is there no way that the original exterior fill request could be reworked to be acceptable?

Contributor

nyalldawson commented May 23, 2014

@mhugo - hmmm... if there's no way fill renderers can be notified that they are being used as a mask, then it effectively writes off the possibility of exterior shapeburst fills, which is a real shame! @wonder-sk do you have any ideas how this could be achieved? is there no way that the original exterior fill request could be reworked to be acceptable?

@wonder-sk

This comment has been minimized.

Show comment
Hide comment
@wonder-sk

wonder-sk May 23, 2014

Member

@mhugo maybe "exterior fill" would be better name? or XOR renderer :-)

@nyalldawson I am not sure if I follow... why there must be a way for shapeburst fill to be notified if used for exterior? If necessary, we could add such hint to the symbol layer, so that e.g. also preview is automatically generated with exterior. If it is just in order to make impression of geometry spanning the whole plane, maybe the exterior ring of the geometry can be set in a way that its width/height is several times greater than the actual window, leading to an impression that it is infinite.

I would prefer not to go back to the previous design as it introduced several changes there were making the rendering process a bit too complex, additionally the support for external fill had to be implemented separately for each fill symbol layer type. I understand your concerns about the fact that a layer would need to be in project twice just to overcome some technical limitations - we could try to discuss how the rendering behaviour could be modified to handle also more complex scenarios. There is already some added complexity because of symbol levels - in other pieces of software, one has to load a layer multiple times for such effect. I believe this renderer is not going to be used on daily basis, so I would consider this to be an acceptable annoyance (and duplication of a layer is just two clicks away).

Member

wonder-sk commented May 23, 2014

@mhugo maybe "exterior fill" would be better name? or XOR renderer :-)

@nyalldawson I am not sure if I follow... why there must be a way for shapeburst fill to be notified if used for exterior? If necessary, we could add such hint to the symbol layer, so that e.g. also preview is automatically generated with exterior. If it is just in order to make impression of geometry spanning the whole plane, maybe the exterior ring of the geometry can be set in a way that its width/height is several times greater than the actual window, leading to an impression that it is infinite.

I would prefer not to go back to the previous design as it introduced several changes there were making the rendering process a bit too complex, additionally the support for external fill had to be implemented separately for each fill symbol layer type. I understand your concerns about the fact that a layer would need to be in project twice just to overcome some technical limitations - we could try to discuss how the rendering behaviour could be modified to handle also more complex scenarios. There is already some added complexity because of symbol levels - in other pieces of software, one has to load a layer multiple times for such effect. I believe this renderer is not going to be used on daily basis, so I would consider this to be an acceptable annoyance (and duplication of a layer is just two clicks away).

@mhugo

This comment has been minimized.

Show comment
Hide comment
@mhugo

mhugo May 23, 2014

Contributor

One can take a rectangle multiple times greater than the extent to simulate infinity. There is still another problem with border styles. For any multiple you choose, there always can be a border width that would be visible. Choosing a big multiple should be ok, but not perfect.
Another problem I am now facing : when on-the-fly projection is enabled, projection of the extent is not always computable, and even less when the extent is bigger ...
I am thinking about a simple way to mark a polygon as an infinite one and postpone the coordinate computation (in renderPolygon basically) ... using perhaps (-1,-1) coordinates ?
We are mixing real coordinates and coordinates that only serve during the rendering here ...

Contributor

mhugo commented May 23, 2014

One can take a rectangle multiple times greater than the extent to simulate infinity. There is still another problem with border styles. For any multiple you choose, there always can be a border width that would be visible. Choosing a big multiple should be ok, but not perfect.
Another problem I am now facing : when on-the-fly projection is enabled, projection of the extent is not always computable, and even less when the extent is bigger ...
I am thinking about a simple way to mark a polygon as an infinite one and postpone the coordinate computation (in renderPolygon basically) ... using perhaps (-1,-1) coordinates ?
We are mixing real coordinates and coordinates that only serve during the rendering here ...

@mhugo

This comment has been minimized.

Show comment
Hide comment
@mhugo

mhugo May 23, 2014

Contributor

The problem with on-the-fly reprojection is now fixed: the exterior extent coordinates are computed directly in the destination CRS and reprojection is done in renderFeature() (after disabling coordinate transformation in the rendering context). This way we can have a square geometry that covers all the windows, even if it has invalid coordinates.
Added SIP and some unit tests as well to complete the PR.
@wonder-sk or @nyalldawson can you merge it before the feature freeze ?

Contributor

mhugo commented May 23, 2014

The problem with on-the-fly reprojection is now fixed: the exterior extent coordinates are computed directly in the destination CRS and reprojection is done in renderFeature() (after disabling coordinate transformation in the rendering context). This way we can have a square geometry that covers all the windows, even if it has invalid coordinates.
Added SIP and some unit tests as well to complete the PR.
@wonder-sk or @nyalldawson can you merge it before the feature freeze ?

wonder-sk added a commit that referenced this pull request May 24, 2014

Merge pull request #1380 from Oslandia/mask_renderer
[FEATURE] Inverted polygons renderer

@wonder-sk wonder-sk merged commit 0775a89 into qgis:master May 24, 2014

@wonder-sk

This comment has been minimized.

Show comment
Hide comment
@wonder-sk

wonder-sk May 24, 2014

Member

@jef-n sorry for merging this slightly after the official feature freeze

Member

wonder-sk commented May 24, 2014

@jef-n sorry for merging this slightly after the official feature freeze

@nyalldawson

This comment has been minimized.

Show comment
Hide comment
@nyalldawson

nyalldawson May 24, 2014

Contributor

@mhugo - thinking about this, it should be sufficient to make sure the outer ring has a margin of at least the canvas width/height. That should be enough to fix the shapeburst fill...

Contributor

nyalldawson commented May 24, 2014

@mhugo - thinking about this, it should be sufficient to make sure the outer ring has a margin of at least the canvas width/height. That should be enough to fix the shapeburst fill...

@nirvn

This comment has been minimized.

Show comment
Hide comment
@nirvn

nirvn May 24, 2014

Contributor

@mhugo @wonder-sk @nyalldawson I've spotted an issue with overlapping polygons (seems the polygons are merged using a symmetrical difference method, which is wrong), see http://hub.qgis.org/issues/10338 for description & test project.

Contributor

nirvn commented May 24, 2014

@mhugo @wonder-sk @nyalldawson I've spotted an issue with overlapping polygons (seems the polygons are merged using a symmetrical difference method, which is wrong), see http://hub.qgis.org/issues/10338 for description & test project.

@nyalldawson

This comment has been minimized.

Show comment
Hide comment
@nyalldawson

nyalldawson May 25, 2014

Contributor

@mhugo I've been thinking about this and have come up with an alternate method. Given the shortcomings with the current approach:

  • Behaviour is undefined for multiple polygons when they overlap (looks difficult to solve)
  • Impossible to use as part of symbol levels
  • Impossible to use with rule-based renderer

A simple alternative method would be to use a similar approach to the initial first exterior fill pull request, but instead of inverting the polygons when the render ends instead invert each polygon individually as it is rendered. This has the benefits of:

  • Can be used as a symbol level along with non-exterior fill symbol levels
  • Can be used in a rule based renderer, so that eg only the current atlas feature is rendered as an exterior fill, and the rest rendered as transparent polygons
  • Individual renderers (eg shapeburst) can have special handling for the exterior fill option
  • Avoids the need for a new rendering type, and exterior fills can be used during categorised renderers and with all the other wonderful symbology features QGIS has.

Possible downsides of this approach are:

  • Behaviour may be unexpected for overlapping polygons - in this case, each polygon will have an exterior fill drawn. Given that the current approach is broken for multiple polygons anyway and that the major use case for exterior fills is for rendering single features, this isn't a significant negative. A tooltip for the exterior fill checkbox explaining the behaviour, eg "Inverts each polygon before rendering" would help overcome this too.
  • Each fill renderer would need to have an exterior fill checkbox and handling added. Given that the change is fairly simple (adding an exterior ring for the canvas bounds) this isn't a large amount of work.

@mhugo @wonder-sk @nirvn What do you think?

Contributor

nyalldawson commented May 25, 2014

@mhugo I've been thinking about this and have come up with an alternate method. Given the shortcomings with the current approach:

  • Behaviour is undefined for multiple polygons when they overlap (looks difficult to solve)
  • Impossible to use as part of symbol levels
  • Impossible to use with rule-based renderer

A simple alternative method would be to use a similar approach to the initial first exterior fill pull request, but instead of inverting the polygons when the render ends instead invert each polygon individually as it is rendered. This has the benefits of:

  • Can be used as a symbol level along with non-exterior fill symbol levels
  • Can be used in a rule based renderer, so that eg only the current atlas feature is rendered as an exterior fill, and the rest rendered as transparent polygons
  • Individual renderers (eg shapeburst) can have special handling for the exterior fill option
  • Avoids the need for a new rendering type, and exterior fills can be used during categorised renderers and with all the other wonderful symbology features QGIS has.

Possible downsides of this approach are:

  • Behaviour may be unexpected for overlapping polygons - in this case, each polygon will have an exterior fill drawn. Given that the current approach is broken for multiple polygons anyway and that the major use case for exterior fills is for rendering single features, this isn't a significant negative. A tooltip for the exterior fill checkbox explaining the behaviour, eg "Inverts each polygon before rendering" would help overcome this too.
  • Each fill renderer would need to have an exterior fill checkbox and handling added. Given that the change is fairly simple (adding an exterior ring for the canvas bounds) this isn't a large amount of work.

@mhugo @wonder-sk @nirvn What do you think?

@nirvn

This comment has been minimized.

Show comment
Hide comment
@nirvn

nirvn May 25, 2014

Contributor

It seems we have three proposals to choose from:
[A] the Inverted Polygons symbology (as committed to qgis master)
[B] the above proposal by @nyalldawson
[C] the original proposal by @mhugo

If I'm not wrong, the main goal we are trying to achieve here revolves around styling of exterior area of atlas features.

Table of what's possible for each option.

Ability Option A Option B Option C
rule-based $atlasfeature no yes yes
handles multiple polygons yes no yes
respects layer order yes yes no
handles overlapping polygons no no no

On that basis, I feel option A isn't a suitable candidate. As it's currently implemented, it is impossible to style a layer using the inverted polygons based on an $atlasfeature expression / rule, defeating the main goal we're trying to achieve. (I also don't think relying on a plugin to set the layer's SQL filter is an acceptable solution)

We're left with option B and option C, and my vote personally goes to option C, for its the one that opens the door to more possibilities (i.e. support for multiple non-overlapping polygons). Its main downside is that it does not respect the layer order (which is actually why @mhugo 's initial proposal was revisited), but it seems all of the alternatives are less capable / suitable.

Option C's lack of respect for the layer stacking / order could be dealt with by placing a label to inform the user of the unusual behavior.

Thoughts @mhugent @wonder-sk @nyalldawson ?

Contributor

nirvn commented May 25, 2014

It seems we have three proposals to choose from:
[A] the Inverted Polygons symbology (as committed to qgis master)
[B] the above proposal by @nyalldawson
[C] the original proposal by @mhugo

If I'm not wrong, the main goal we are trying to achieve here revolves around styling of exterior area of atlas features.

Table of what's possible for each option.

Ability Option A Option B Option C
rule-based $atlasfeature no yes yes
handles multiple polygons yes no yes
respects layer order yes yes no
handles overlapping polygons no no no

On that basis, I feel option A isn't a suitable candidate. As it's currently implemented, it is impossible to style a layer using the inverted polygons based on an $atlasfeature expression / rule, defeating the main goal we're trying to achieve. (I also don't think relying on a plugin to set the layer's SQL filter is an acceptable solution)

We're left with option B and option C, and my vote personally goes to option C, for its the one that opens the door to more possibilities (i.e. support for multiple non-overlapping polygons). Its main downside is that it does not respect the layer order (which is actually why @mhugo 's initial proposal was revisited), but it seems all of the alternatives are less capable / suitable.

Option C's lack of respect for the layer stacking / order could be dealt with by placing a label to inform the user of the unusual behavior.

Thoughts @mhugent @wonder-sk @nyalldawson ?

@nirvn

This comment has been minimized.

Show comment
Hide comment
@nirvn

nirvn May 25, 2014

Contributor

Option C allows for such styling of non-overlapping multiple polygons, which I think is worth fighting for :-)
1978454_10152297806289916_732871874_o

The above pictured land concession polygons could very well be islands and seacoast, etc. Notice how the two polygons on the right side with a shared edge have the exterior shapeburst merged together and not appear under each other's polygon? That can only be achieved via Option C.

Contributor

nirvn commented May 25, 2014

Option C allows for such styling of non-overlapping multiple polygons, which I think is worth fighting for :-)
1978454_10152297806289916_732871874_o

The above pictured land concession polygons could very well be islands and seacoast, etc. Notice how the two polygons on the right side with a shared edge have the exterior shapeburst merged together and not appear under each other's polygon? That can only be achieved via Option C.

@nirvn

This comment has been minimized.

Show comment
Hide comment
@nirvn

nirvn May 25, 2014

Contributor

Thinking outside of the box for a minute, here's another option: creating a root 'exterior fill' node in the symbology layers list, in which the user can add as many layers as he/she wants. That would take care of option C's main flaw (not following layer order), and actually prevent the need for a [x] exterior fill checkbox in each symbology style (simple fill, shapeburst, pattern fill, etc.)

It could like this:
outside_of_the_box

Let's call this option D.

Criteria Option A Option B Option C Option D
rule-based $atlasfeature no yes yes yes
handles multiple polygons yes no yes yes
respects layer order yes yes no yes
no additional [x] exterior fill checkbox requirement yes no no yes
handles overlapping polygons no no no no
Contributor

nirvn commented May 25, 2014

Thinking outside of the box for a minute, here's another option: creating a root 'exterior fill' node in the symbology layers list, in which the user can add as many layers as he/she wants. That would take care of option C's main flaw (not following layer order), and actually prevent the need for a [x] exterior fill checkbox in each symbology style (simple fill, shapeburst, pattern fill, etc.)

It could like this:
outside_of_the_box

Let's call this option D.

Criteria Option A Option B Option C Option D
rule-based $atlasfeature no yes yes yes
handles multiple polygons yes no yes yes
respects layer order yes yes no yes
no additional [x] exterior fill checkbox requirement yes no no yes
handles overlapping polygons no no no no
@mhugo

This comment has been minimized.

Show comment
Hide comment
@mhugo

mhugo May 26, 2014

Contributor

@nyalldawson @nirvn I am a bit lost ...
The inverted polygon renderer do work with a rule-based sub renderer : one inverted polygon will be built for each "symbol category" found in the sub renderer. So for instance, if you have a rule-based renderer with $atlasfeatureid = $id and an "else" rule, there will be two categories and then two inverted polygons with two different styles rendered. Are you expecting something else or different here ?
@nyalldawson I don't understand what do you mean by inverting each polygon individually. Suppose you have two polygon features in a layer with a 'single' fill pattern and you want to inverse them : what will be the result ? If I invert them individually, there will be two 'inside-out' polygons overlapping each other, so there will only be one polygon visible. I am not sure this is what we want. Please correct me if I missed something.

About the overlapping polygons, this is something that would have to be fixed whatever the approach (I realize it also applied to my first proposal).

My initial proposal do respect symbol layers, but at a price that is too high in terms of code complexity.
@nirvn one problem was the need for a checkbox for each filling style, but also a need for a particular handling code for each filling class. On a GUI side, your proposal makes sense, but I am not sure this will change anything regarding the code complexity (I will try to have a look though).

Contributor

mhugo commented May 26, 2014

@nyalldawson @nirvn I am a bit lost ...
The inverted polygon renderer do work with a rule-based sub renderer : one inverted polygon will be built for each "symbol category" found in the sub renderer. So for instance, if you have a rule-based renderer with $atlasfeatureid = $id and an "else" rule, there will be two categories and then two inverted polygons with two different styles rendered. Are you expecting something else or different here ?
@nyalldawson I don't understand what do you mean by inverting each polygon individually. Suppose you have two polygon features in a layer with a 'single' fill pattern and you want to inverse them : what will be the result ? If I invert them individually, there will be two 'inside-out' polygons overlapping each other, so there will only be one polygon visible. I am not sure this is what we want. Please correct me if I missed something.

About the overlapping polygons, this is something that would have to be fixed whatever the approach (I realize it also applied to my first proposal).

My initial proposal do respect symbol layers, but at a price that is too high in terms of code complexity.
@nirvn one problem was the need for a checkbox for each filling style, but also a need for a particular handling code for each filling class. On a GUI side, your proposal makes sense, but I am not sure this will change anything regarding the code complexity (I will try to have a look though).

@nirvn

This comment has been minimized.

Show comment
Hide comment
@nirvn

nirvn May 26, 2014

Contributor

@mhugo I had totally missed the sub renderer option within the inverted polygons panel, that helps opening the door to new possibilities.

So what this means is that the only one thing we can't do with this implementation is mixing up exterior / inverted styling and interior / normal styling of polygons within one layer. That's something I can live with. @nyalldawson ?

As for the overlapping polygons, IMO it's serious enough to warrant some fix. If you think a polygons' union method is too costly by default, might be worth to add it as an option via a [x] checkbox above or below the sub renderer row? The default could be the current fast method, with an alternative slow path of union.

Contributor

nirvn commented May 26, 2014

@mhugo I had totally missed the sub renderer option within the inverted polygons panel, that helps opening the door to new possibilities.

So what this means is that the only one thing we can't do with this implementation is mixing up exterior / inverted styling and interior / normal styling of polygons within one layer. That's something I can live with. @nyalldawson ?

As for the overlapping polygons, IMO it's serious enough to warrant some fix. If you think a polygons' union method is too costly by default, might be worth to add it as an option via a [x] checkbox above or below the sub renderer row? The default could be the current fast method, with an alternative slow path of union.

@nirvn

This comment has been minimized.

Show comment
Hide comment
@nirvn

nirvn May 26, 2014

Contributor

@mhugo in the above posted screenshot, I'm actually showing off a mix exterior + inside styling (the polygons have a line pattern inside)

Contributor

nirvn commented May 26, 2014

@mhugo in the above posted screenshot, I'm actually showing off a mix exterior + inside styling (the polygons have a line pattern inside)

@nirvn

This comment has been minimized.

Show comment
Hide comment
@nirvn

nirvn May 26, 2014

Contributor

So I'm able to do rule-based exterior styling (see below), but as mentioned above can't mix exterior and interior styling (i.e. I can't put ///// inside the polygons).

rule-exterior

Contributor

nirvn commented May 26, 2014

So I'm able to do rule-based exterior styling (see below), but as mentioned above can't mix exterior and interior styling (i.e. I can't put ///// inside the polygons).

rule-exterior

@mhugo

This comment has been minimized.

Show comment
Hide comment
@mhugo

mhugo May 26, 2014

Contributor

@nirvn Yes exactly. If you want to add an interior //// pattern, you would have to duplicate the layer and the rule with the interior filling

Contributor

mhugo commented May 26, 2014

@nirvn Yes exactly. If you want to add an interior //// pattern, you would have to duplicate the layer and the rule with the interior filling

@nirvn

This comment has been minimized.

Show comment
Hide comment
@nirvn

nirvn May 26, 2014

Contributor

@mhugo, ok.

BTW, filed a crasher against this feature: http://hub.qgis.org/issues/10355

Contributor

nirvn commented May 26, 2014

@mhugo, ok.

BTW, filed a crasher against this feature: http://hub.qgis.org/issues/10355

@nyalldawson

This comment has been minimized.

Show comment
Hide comment
@nyalldawson

nyalldawson May 26, 2014

Contributor

@mhugo - ahh... i'd also missed that "sub renderer" option. That missing feature was my biggest concern. Thank you! All I'd like to see now is the canvas bounds outer ring be extended to the width/height of the canvas from the edges to fix the shapeburst fill...

Contributor

nyalldawson commented May 26, 2014

@mhugo - ahh... i'd also missed that "sub renderer" option. That missing feature was my biggest concern. Thank you! All I'd like to see now is the canvas bounds outer ring be extended to the width/height of the canvas from the edges to fix the shapeburst fill...

@mhugo

This comment has been minimized.

Show comment
Hide comment
@mhugo

mhugo May 26, 2014

Contributor

@nyalldawson the current code actually creates an outer ring of 10 times the canvas size.

Contributor

mhugo commented May 26, 2014

@nyalldawson the current code actually creates an outer ring of 10 times the canvas size.

@nyalldawson

This comment has been minimized.

Show comment
Hide comment
@nyalldawson

nyalldawson May 29, 2014

Contributor

@mhugo... Ahhh, I see.. The problem isn't in the mask renderer, its the polygon clipping which occurs automatically - http://hub.qgis.org/issues/9757

Contributor

nyalldawson commented May 29, 2014

@mhugo... Ahhh, I see.. The problem isn't in the mask renderer, its the polygon clipping which occurs automatically - http://hub.qgis.org/issues/9757

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