Skip to content

Fix blanking fill #8107

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Fix blanking fill #8107

wants to merge 4 commits into from

Conversation

pmconne
Copy link
Member

@pmconne pmconne commented May 15, 2025

Fixes #8101.
Dunno why we ever tried to do this using the old-fashioned polygon offset thing when we have fragment shader logic for controlling display priority.
Unit tests proved infeasible.

  • Run ImageTests
  • Fix area pattern symbols z-fighting with blanking region.
  • Determine if differences in raster view attachments are desirable.
  • Address z-fighting at borders of image graphics in sheet title blocks.

@pmconne pmconne requested review from bbastings, aruniverse, ben-polinsky and a team as code owners May 15, 2025 20:55
@pmconne
Copy link
Member Author

pmconne commented May 16, 2025

Area patterns now z-fight with their background shapes. The shapes draw with blanking fill. We test for "same element" in shader by comparing feature Ids. But feature Ids will differ because the pattern geometry is of GeometryClass.Pattern. Same would technically occur if e.g. blanking region was construction geometry with geometry of class primary intended to draw in front of it. It would also happen if the blanking region and other geometry were on different subcategories.

@pmconne
Copy link
Member Author

pmconne commented May 20, 2025

Fixing this requires the fragment shader to be able to answer the question "am I drawing the same element as the one that was previously written to the pick buffer?" This is only relevant for blanking fill - edges always have the exact same feature Id as their surfaces, so the current logic works; and planar regions draw on top of non-planar surfaces regardless of feature/element.

Proposed solution

If a Batch contains any blanking regions, then we need to populate an extra texture that correlates feature index to "element index" (just some unique integer that is the same for all features belonging to the same element - e.g., the index of that element in a list of unique elements in the batch, or the index of the first feature associated with that element).
When drawing the linear and planar passes, the fragment shader looks up the element index and outputs it to the pick buffer.

When drawing the non-planar pass (in which planar blanking regions are now drawn), the fragment shader looks up the element index of the current feature and compares it to the one in the pick buffer. If and only if they match, the blanking region is discarded in favor of the previously-drawn fragment.

A couple of limitations:

  • WebGL 2 guarantees a minimum of 16 texture units, no more. We recently allocated all 6 of the previously-free units for draping map layers onto iModel geometry. I propose we take one of those back to be used for the element index lookup texture.
  • WebGL 2 guarantees a minimum of 4 fragment shader outputs. We recently allocated the 4th one for contour line information. It is extremely unlikely that blanking fill and contour lines will both be relevant for a given fragment, and in any case, contour lines recolor the fragment, making blanking fill unimportant. I propose that if a fragment is not a contour line, then we output the element index to the fragment shader output that otherwise would hold contour line info. We would need to ensure the contour buffer gets ping-ponged like the other pick buffer outputs.

@markschlosseratbentley I'm open to other ideas.

Priority

I'm inclined to defer implementation temporarily as we have more pressing problems in the area of drawing production (and elsewhere). I suspect that in general users try to avoid having text with background fill overlap other geometry in their section drawings. @Josh-Schifter what is your take on the urgency?

@Josh-Schifter
Copy link
Contributor

I suspect that in general users try to avoid having text with background fill overlap other geometry in their section drawings.

I suspect that users try to avoid overlapping, but based on what I'm seeing from example site design sheets it is often unavoidable. Here's some screenshots.

image

image

image

image

@pmconne
Copy link
Member Author

pmconne commented May 20, 2025

I suspect that users try to avoid overlapping, but based on what I'm seeing from example site design sheets it is often unavoidable. Here's some screenshots.

Thanks for the examples. In that case I think @claudiaareneee should proceed with #8055 with the assumption that we will address the blanking fill issue separately. @markschlosseratbentley if no one else picks it up in the meanwhile I should be able to get back to it in a week or so.

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.

Blanking fill draws behind other elements
4 participants