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

filter() does not work after 1.9.0 if noFill() is called #7425

Open
inaridarkfox4231 opened this issue Dec 15, 2024 · 10 comments
Open

filter() does not work after 1.9.0 if noFill() is called #7425

inaridarkfox4231 opened this issue Dec 15, 2024 · 10 comments

Comments

@inaridarkfox4231
Copy link
Contributor

Topic

filter() was introduced in webgl mode in 1.8.0. At this time, it seems that filter() was still working even with noFill().

// version: 1.8.0
function setup() {
   createCanvas(200,200,WEBGL);
 }
  
 function draw() {
   background(0);
   noFill();
   stroke(255);
   line(-100,-100,300,300);
   filter(INVERT);
 }

cwcdcwvr3v3

However, since 1.9.0, filter() does not work if noFill() is called. Of course it doesn't work with 1.11.2 either.

r3f3f33f33f

It will be drawn when you execute fill().

function setup() {
    createCanvas(200,200,WEBGL);
}

function draw() {
    background(0);
    noFill();
    stroke(255);
    line(-100,-100,300,300);
    
    fill(255);
    
    filter(INVERT);
}

ef3d3frf3f3f3

It seems nonsensical to have to call pointless fill() to filter a sketch that just draws lines.
But I don't know if this is a bug or not.

That's all for raising the issue.

@subhraneel2005
Copy link

hi, i checked it in the web editor, and it seems actually pointless to call the fill() unneccessary. i will be working on this then

just let me know that you get this bug in the p5.js web editor?

@inaridarkfox4231
Copy link
Contributor Author

here: noFill_filter_BUG

noFill() and no "fill()"

function setup() {
  createCanvas(400, 400, WEBGL);

  rotateX(PI/4);
  rotateY(PI/4);
  noFill();
  stroke(255);
  background(0);
  box(200);

  filter(INVERT);
}

hjhgwhdghvfbh

noFill() and "fill()"

function setup() {
  createCanvas(400, 400, WEBGL);

  rotateX(PI/4);
  rotateY(PI/4);
  noFill();
  stroke(255);
  background(0);
  box(200);

  fill("aquamarine");

  filter(INVERT);
}

efefefrf3ffr4f4

@subhraneel2005
Copy link

yes, if nofill() is given then giving fill() is uneccesary, thanks for noticing it. will work on this

@inaridarkfox4231
Copy link
Contributor Author

Thank you for your working on this issue. I'm not familiar with the behavior around filter() and framebuffer, so I can't work for this issue.

@subhraneel2005
Copy link

its all good, no worries 😊

@subhraneel2005
Copy link

i think the bug was with noFill().
just added this._renderer.fill(255); to it. So, it will have a white fill by default, and we dont have to call fill() again. what do u think ?
sol1

@davepagurek
Copy link
Contributor

Just a heads up -- In the dev-2.0 branch, where p5 2.0 is being developed, this section of the code has been refactored, so the implementation will likely be a little different. We now store the fill color as _renderer.states.fillColor, which is null if there should not be a fill, and is a p5.Color if a fill is active.

@subhraneel2005
Copy link

subhraneel2005 commented Dec 18, 2024

alright, then according to your insights, the code should be like this:

p5.prototype.noFill = function() {  
  this._renderer.states.fillColor = null; 
  return this;  
};  

please correct me if im wrong

@Vaivaswat2244
Copy link

Vaivaswat2244 commented Mar 5, 2025

Is someone working on this? If not I'd like to proceed on this.
Also instead of providing the fillColor with null. I'm assuming making it transparent white would be a better fix,
something on the lines of,

p5.prototype.noFill = function() {  
  this._renderer.states.fillColor = this.color(255, 255, 255, 0); 
  return this;  
};

@davepagurek
Copy link
Contributor

There's a performance (and depth ordering) implication to having fill set, since we have to triangulate all the faces in order to fill them, and then it has an entry in the depth buffer, preventing farther-away objects from being drawn at those pixels. So I think we still want to avoid doing fills at all if noFill has been set, but ideally just update the filter() logic to run regardless of the fill.

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

No branches or pull requests

4 participants