#28783 closed enhancement (fixed)

fix opacity for add_condition in mono-coloured plot3d

Reported by: chapoton Owned by:
Priority: major Milestone: sage-9.0
Component: graphics Keywords:
Cc: tscrim, kcrisman, vklein Merged in:
Authors: Frédéric Chapoton Reviewers: Karl-Dieter Crisman
Report Upstream: N/A Work issues:
Branch: fde06be (Commits, GitHub, GitLab) Commit: fde06be72bb5613443a10f625ed6f115ad75c32f
Dependencies: Stopgaps:

Status badges

Change History (13)

comment:1 Changed 22 months ago by chapoton

  • Branch set to u/chapoton/28783
  • Commit set to 943c908cac6a4b41dd21ef3e5aaa15d33f5611a5
  • Status changed from new to needs_review

New commits:

943c908fix opacity for add_condition of mono-coloured plot3d

comment:2 Changed 22 months ago by chapoton

  • Cc tscrim kcrisman vklein added

green bot, please review

comment:3 follow-up: Changed 22 months ago by kcrisman

I like this. A few questions:

  • Are there any other attributes of Texture that may go missing in a similar way? Might as well get them all at once.
  • In particular, where does texture.color go - presumably it's used elsewhere?
  • Can you think of a relevant doctest (perhaps from the ask.sagemath question) that could be added?

comment:4 in reply to: ↑ 3 ; follow-up: Changed 22 months ago by chapoton

Replying to kcrisman:

I like this. A few questions:

  • Are there any other attributes of Texture that may go missing in a similar way? Might as well get them all at once.

No idea, and no time to check. I do not think so, I would say.

  • In particular, where does texture.color go - presumably it's used elsewhere?

Well, it's used everywhere. The Texture object that is transmitted after cutting the surface contains both the color and the opacity. For some reason the opacity is not fetched from the Texture but needs to be given through an additional parameter.

  • Can you think of a relevant doctest (perhaps from the ask.sagemath question) that could be added?

Well, one could test Q.texture.opacity after the cutting, sure.

comment:5 in reply to: ↑ 4 Changed 22 months ago by kcrisman

  • Can you think of a relevant doctest (perhaps from the ask.sagemath question) that could be added?

Well, one could test Q.texture.opacity after the cutting, sure.

That sounds pretty reasonable, then.

comment:6 Changed 22 months ago by git

  • Commit changed from 943c908cac6a4b41dd21ef3e5aaa15d33f5611a5 to ad26e751eddec1f0b5b4eb76021e79ae2bc6cc2f

Branch pushed to git repo; I updated commit sha1. New commits:

ad26e75trac 28783 adding a doctest

comment:7 Changed 22 months ago by git

  • Commit changed from ad26e751eddec1f0b5b4eb76021e79ae2bc6cc2f to fde06be72bb5613443a10f625ed6f115ad75c32f

Branch pushed to git repo; I updated commit sha1. New commits:

fde06betrac 28783 better doctest for opacity

comment:8 Changed 22 months ago by chapoton

ok, here it is. The added doctest would fail before the branch, and passes after.

comment:9 Changed 22 months ago by kcrisman

There is something weird going on, at least with the Sage cell server running 8.9. This test shows that sometimes we only get the bug when things are plotted together. Further, this example shows that the jsmol (at least on Sage cell server) seems not to be affected!


New commits:

fde06betrac 28783 better doctest for opacity

comment:10 Changed 22 months ago by kcrisman

Agreed that the new doctest is better and does capture the issue. Unfortunately the other platform-dependent thing probably should be sorted out - or at least mentioned as "known bug" or something if you think they are somewhat orthogonal and deserve a different ticket. Although I cannot test it on my local install now this should be the right solution.

comment:11 Changed 22 months ago by chapoton

I would say that any other problem should go to another ticket.

comment:12 Changed 22 months ago by kcrisman

  • Reviewers set to Karl-Dieter Crisman
  • Status changed from needs_review to positive_review

If I run into them (or someone else does) with more detail, ok. This ticket seems good to go, though as I say I can't test its efficacy in viewing it in a local build.

comment:13 Changed 22 months ago by vbraun

  • Branch changed from u/chapoton/28783 to fde06be72bb5613443a10f625ed6f115ad75c32f
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.