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

A dream about fbos summing using += operator #7130

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

Conversation

dimitre
Copy link
Member

@dimitre dimitre commented Oct 6, 2022

Last night I dreamt about OF, ofFbo had a += operator so you could sum layers like this.
In fact this still makes sense to me, so I decided to make this quick PR

fbo += fbo2;

example

#include "ofApp.h"

ofFbo fbo;
ofFbo fbo2;
ofFbo fbo3;

//--------------------------------------------------------------
void ofApp::setup(){
	fbo.allocate(600, 600, GL_RGBA);
	fbo.begin();
	ofClear(0,0);
	ofSetColor(255, 0, 0);
	ofDrawCircle(300, 300, 200);
	fbo.end();
	
	fbo2.allocate(600, 600, GL_RGBA);
	fbo2.begin();
	ofClear(0,0);
	ofSetColor(0, 0, 255);
	ofDrawRectangle(100, 100, 300, 50);
	fbo2.end();

	fbo3.allocate(600, 600, GL_RGBA);
	fbo3.begin();
	ofClear(0,0);
	ofSetColor(0, 255, 0);
	ofDrawRectangle(200, 200, 300, 50);
	fbo3.end();

	fbo += fbo2;
	fbo += fbo3;
}

//--------------------------------------------------------------
void ofApp::update(){

}

//--------------------------------------------------------------
void ofApp::draw(){
	fbo.draw(0,0);
}

void ofApp::keyPressed(int key) {

}

Screen Shot 2022-10-06 at 16 23 03

@ofTheo
Copy link
Member

ofTheo commented Oct 6, 2022

love this idea.
curious what others think? @ofZach @NickHardeman @2bbb

I would definitely consider removing the ofSetColor in the += incase you would want to do that explicitly.

@dimitre
Copy link
Member Author

dimitre commented Oct 6, 2022

yeah I was in doubt about setcolor, I'll remove it now

@NickHardeman
Copy link
Contributor

I like the idea of operators.
It makes sense using GL_RGBA, but a GL_RGB fbo would just draw right over the other. Not sure if this would be confusing or not.
I would also be interested in an equal operator to a color like fbo = ofColor::black that simply begins, clears to color and then ends.

@2bbb
Copy link
Contributor

2bbb commented Oct 31, 2022

It seems like good!

Can we support more general rhs targets like ofImage, ofVideoPlayer, ofVideoGrabber?
(or more and more general, structs have draw method...?😊)

Yes, I know, making thing general makes complex.
However, I think it would be good if it could be generalized with a good balance.

@ofZach
Copy link
Contributor

ofZach commented Oct 31, 2022

one question -- what happens when FBOs are not the same size?

am a fan of the = ofColor approach that @NickHardeman suggested.

@dimitre
Copy link
Member Author

dimitre commented Oct 31, 2022

@ofZach it can be drawin in 0,0 coordinate. top left

@dimitre
Copy link
Member Author

dimitre commented Oct 31, 2022

I like the ofColor operator too!

@dimitre
Copy link
Member Author

dimitre commented Oct 31, 2022

@NickHardeman I think it is OK to just overwrite if there is no transparency or blend mode enabled for the two images.
Not an unexpected result.

@2bbb
Copy link
Contributor

2bbb commented Nov 1, 2022

This is just an idea.

+= is simple and understandable.
But fbo += fbo1 and fbo = fbo + fbo1 are not same. (I don't know there is good implementation for fbo + fbo1)
Maybe this is not so important. However, there is a point of concern.

And I think design of iostream is not so cool. (mainly about string formatting and iomanip...)
However I think those style is possibility those are better and extendable way to fbo processing.

like below:

fbo << fbo1;
fbo << fbo2 << fbo3;
fbo << ofFboOpStyle::Fill << fbo4;
fbo << ofFboOpStyle::AspectFit << fbo5;
fbo << ofFboOpStyle::Offset(10, 10) << fbo6;

I know those may be over-featured or non-intuitive...
As a mere suggestion.

@dimitre
Copy link
Member Author

dimitre commented Dec 7, 2022

I really like this one
I don't see any downside since += operator is not very obvious to use in objects
but makes sense in this fbo case.

f.draw(0,0);
this->end();
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

might want to add a check that if the fbo isn't allocated you can an error/warning
or you could have if it is not allocated it gets allocated to match first?

@@ -302,6 +302,13 @@ ofFbo::ofFbo(const ofFbo & mom){
}
}

//--------------------------------------------------------------
void ofFbo::operator+=(const ofFbo & f){
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

arg should be fbo in header/cpp I think to be consistent.

this->begin();
f.draw(0,0);
fbo.draw(0,0);
this->end();
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feel like it should be something like:

	//if we aren't allocated lets allocate to be the same as the input
	if (!this->isAllocated() && fbo.isAllocated()) {
 		this->allocate(fbo.settings); 
 	}
	if (fbo.isAllocated()) {
 		this->begin();
 		fbo.draw(0,0);
 		this->end();
 	}else{
 		ofLogWarning("ofFbo::operator+=") << " input fbo is not allocated, skipping += ";
 	}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dimitre what do you think about the suggestion above?👆

This way it basically copies the fbo being += if the main fbo isn't allocated

@artificiel
Copy link
Contributor

artificiel commented Feb 19, 2023

what about fbo *= shader, where fbo is passed through shader, transparently swapping a lazily-allocated buffer?

[edit: of course I'm not specifically advocating the *= operator itself (although it feels not bad) but the principle]

@artificiel
Copy link
Contributor

(posting like this as I'm not sure how to pull another user's branch then add to the PR?)

ofFbo.h

	ofFbo & operator *=(const ofShader & shader) { return pass(shader); }
	ofFbo & pass(const ofShader & shader);
	std::unique_ptr<ofFbo> buffer; // sole overhead if method is unused is unallocated pointer

ofFbo.cpp

ofFbo & ofFbo::pass(const ofShader & shader) {
	
	// lazy  --  perhaps augment settings to make within allocate() if so desired
	if (!buffer) buffer = std::make_unique<ofFbo>();
	
	// conformant  --  if compare is too costly at every frame, hook into allocate to maintain sync
	if (buffer->settings != settings) buffer->allocate(settings);
	
	buffer->begin();
	ofClear(0,0,0,0);
	shader.begin();
	draw(0,0);
	shader.end();
	buffer->end();
	
	// perhaps this could be "texture swapped" with some clever GL code?
	begin();
	buffer->draw(0,0);
	end();
	
	return *this; // to enable things like : flatten_fbo += (layer_fbo *= layer_shader);
}

ofApp.cpp:

    fbo *= shader;

@artificiel artificiel mentioned this pull request Aug 31, 2023
36 tasks
@dimitre dimitre added this to the 0.12.1 milestone Aug 31, 2023
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.

6 participants