Screenshot for 3D view #5324
Screenshot for 3D view #5324
Conversation
|
@MichaelBuessemeyer Could you have a look at this? :) |
|
I just tested this feature and it works very well 🎉 While looking at the screenshots, I noticed that rendering the "out of dataset" parts with nothing looks a bit weird. Maybe we could render these parts just the same way they are rendered in the viewports (black)? But this is only a guess whether this looks better. What do you think @daniel-wer? What about a follow-up issue for this? |
@daniel-wer could you please provide me with a bit more details on how to reproduce this bug? I wasn't able to reproduce it in the master. |
Interesting, this seems to be browser-dependent - Chrome rendered the "out of dataset" parts in a yellowish white which was less obtrusive. This is because the "out of dataset" parts get an alpha value of 0 in the shader (which is the cause of the mirroring bug I fixed). We could either change this alpha value in the shader code or initialize the buffer that is rendered into with the correct background color, I think. I'll think about which solution I prefer.
Doing a screenshot so that the upper half of the xy viewport is outside of the dataset bounding box should trigger this bug (I used chrome). The result looked like this:
|
There was a problem hiding this comment.
The code looks good and works in each view. I also could reproduce the bug on the master (using firefox and chrome) and can confirm that it is solved on this branch 🚀
Interesting, this seems to be browser-dependent - Chrome rendered the "out of dataset" parts in a yellowish-white which was less obtrusive. This is because the "out of dataset" parts get an alpha value of 0 in the shader (which is the cause of the mirroring bug I fixed). We could either change this alpha value in the shader code or initialize the buffer that is rendered into with the correct background color, I think. I'll think about which solution I prefer.
I think we should wait for merging until you pushed a solution for this. But if you think this can be done in follow-up PR this is fine too, as the "out of dataset" parts are rendered as before.
frontend/javascripts/libs/utils.js
Outdated
| // Create a new canvas when flipping as otherwise pixels of the unflipped | ||
| // image would "shine through" for pixels which are not opaque when flipping the | ||
| // image in-place |
There was a problem hiding this comment.
Smart solution 🤓 ✔️
| <Menu.Item key="share-button" onClick={() => setShareDatasetModalVisibility(true)}> | ||
| <Icon type="share-alt" /> | ||
| Share | ||
| </Menu.Item> | ||
| {screenshotMenuItem} |
There was a problem hiding this comment.
Nice addition 👍
…ing and transparency issue
| @@ -137,7 +137,7 @@ ${compileShader( | |||
| void main() { | |||
| vec3 worldCoordUVW = getWorldCoordUVW(); | |||
| if (isOutsideOfBoundingBox(worldCoordUVW)) { | |||
| gl_FragColor = vec4(0.0); | |||
| gl_FragColor = vec4(0.0, 0.0, 0.0, 1.0); | |||
There was a problem hiding this comment.
@philippotto Setting the alpha value to 1 for pixels outside of the bounding box, fixes both the mirroring issue (described in the PR description - would make my earlier fix obsolete) as well as the other transparency issue (see Michaels screenshot). Do you see any harm in that? I checked that the node picking is not affected.
There was a problem hiding this comment.
Sounds perfectly fine to me 👍 I don't think that a lot of reasoning went into this line when it was written. The result probably behaved like "black" in the critical use case (viewport is out of bounds) which was considered good enough. Cool that you found this solution :)
There was a problem hiding this comment.
I can't find any case where it would result in a 🐛 too.
|
I retested the changes on the dev deployment and they seem to work in all cases. It should be ready for 🚢 |
Also screenshot the 3D view when using the screenshot functionality.
Allow to screenshot using the menu button in view mode.
During testing I noticed that screenshots near the dataset border included a mirrored version of the actual screenshot. This bug was happening, because the screenshot needs to be flipped horizontally due to the inverted webgl y-axis. For pixels outside of the dataset, the alpha value is set to 0. Therefore, when flipping the image of a canvas in-place and drawing it to the same canvas, you could see-through the unflipped version of the image in some areas. I fixed that by drawing the flipped image to another canvas.
See the followup issue #5323 regarding antialiasing of the screenshots.
URL of deployed dev instance (used for testing):
Steps to test:
Issues: