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

OpenGL intialisatie #22

Merged
merged 14 commits into from
Apr 30, 2018
Merged

OpenGL intialisatie #22

merged 14 commits into from
Apr 30, 2018

Conversation

RoanH
Copy link
Collaborator

@RoanH RoanH commented Apr 28, 2018

OpenGL intialisatie.

Alles is geoptimaliseerd voor het renderen van visualisaties die niet vaak veranderen. Pannen en zoomen telt hierbij niet aangezien dat alleen van toepassing is op de modelview matrix. Tenzij er hierdoor meer content bij komt die voorheen niet gerenderd hoefde te worden.

De implementatie is als volgt:

  • De OpenGL viewport is 1600 pixels bij 900 pixels (16:9).
  • De OpenGL viewport word in het midden van het canvas gecentreerd en zo groot mogelijk gemaakt. Dit betekend dat de randen van de viewport niet altijd zichtbaar zijn. Dus alhoewel de viewport zelf 1600x900 is, is de effectieve viewport de grootte van het canvas. Dit heeft voor onze use case echter ook een aantal voordelen.
  • Dit betekend namelijk dat de visualisatie altijd word geschaald om op het canvas te passen. Hierdoor ziet de visualisatie er altijd hetzelfde uit ongeacht de monitor resolutie / browser grootte.
  • Het tweede voordeel is dat het coördinaten systeem tijdens het visualiseren vast staan. Dus dingen hoeven niet geschaald te worden tijdens het visualiseren. En er kan worden aangenomen dat maximaal 1600x900 zichtbaar is en dat dus content buiten dit rechthoek niet gevisualiseerd hoeft te worden. Ook niet als de user de browser resized.
  • Het enige moment waarop de visualisatie opnieuw berekent hoef te worden is als er door pannen/zoomen naar nieuwe content word genavigeerd die voorheen niet zichtbaar was. Afhankelijk van de visualisatie zou hier eventueel echter ook een incrementeel proces gebruikt kunnen worden.
  • Het coordinaten system is gecentreerd met 0,0 in het midden van de OpenGL viewport. Dit betekend dat als er niet gepand/gezoomed is het zichtbare deel van de visualisatie van -800 tot 800 x en van -450 tot 450 y loopt.
  • De rede dat de viewport 1600x900 is, is omdat standaard de OpenGL viewport voor zowel x als y van -1 tot 1 loopt. Dus afhankelijk van de resolutie van het canvas is een quad van 0.1 bij 0.1 niet altijd vierkant. Door zelf een assen stelsel te definiëren is dit wel zo.

De performance van het renderen is op zich best goed:
De test is uitgevoerd met n quads en met Firefox Quantum 59.0.2 64-bits. Voor elke test is Firefox eerst opnieuw opgestart. De nummers voor RAM zijn RAM gebruik na het laden van de visualisatie - idle RAM gebruik.
n=3, ram=141MB
n=1000, ram=158MB
n=10000, ram=309MB
n=100000, ram=1883MB
n=1000000, ram=17415MB

De webpagina zelf ziet er nu zo uit:
1000000 quads:
afbeelding
13 quads:
afbeelding

fixes #6
Quads zijn ook geïmplementeerd, maar dit zijn axis aligned quads en we hebben nog iets meer nodig als alleen dat.

Ik heb het idee dat er een aantal dingen zijn die ik ben vergeten om te melden, dus als er vragen zijn hoor ik het wel. In de code staan ook redelijk wat comments.

@RoanH RoanH self-assigned this Apr 28, 2018
@RoanH RoanH added this to review in SPRINT 01 Apr 28, 2018
@RoanH RoanH changed the title Feature/opengl OpenGL intialisatie Apr 28, 2018
@RoanH
Copy link
Collaborator Author

RoanH commented Apr 28, 2018

Ik los de MR conflicts later wel even op.

@bartwesselink bartwesselink self-requested a review April 29, 2018 17:27
@bartwesselink
Copy link
Owner

Nette implementatie, mooi om te zien dat de performance goed is. Ik verwacht niet dat we ooit 1 miljoen quads moeten gaan tekenen (te kleine sub-tree), dus ziet er top uit.

Wel is het OpenGL-gedeelte voor mij een beetje abra-cadabra. Dus aan @S4K4YUME ook de vraag hier even naar te kijken.

@NicoKNL
Copy link
Collaborator

NicoKNL commented Apr 29, 2018

@bartwesselink Bij een tree van 500.000 nodes hebben we natuurlijk ook ongeveer 500.000 edges tussen alle nodes (mits we hem als een 'klassieke tree' visualizeren). Als we al die edges als aparte objecten gaan drawen dan komen we natuurlijk wel aan de 1.000.000 .

Stel dat ze een test er tegen aan willen gooien die net wat groter is dan de aangeleverde basis-set op canvas, dan kunnen we daar dus best wel eens tegen aan lopen.

Maar een mogelijke optimalisatie kunnen we het beste naar de toekomst schuiven wat mij betreft.

@RoanH
Copy link
Collaborator Author

RoanH commented Apr 29, 2018

Ook is het zo dat dit een test was met n quads. Elke quad bestaat natuurlijk uit 4 vertices. Dus afhankelijk van de vorm die we renderen kunnen we meer of minder renderen. In het geval van edges zijn er maar 2 vertices per edge, eventueel zelfs minder als we slim de edges renderen.

Ook is het altijd nog mogelijk om een aantal van de buffers vrij te geven na de berekeningen. Echter als we dat doen dan moet de visualisatie gedeeltelijk opnieuw berekend worden voor een repaint event. Feitelijk word de afweging dan RAM usage vs responsiveness.

Verder hebben we nog de optie om te kijken of we VBOs kunnen gebruiken, dit ziet er echter net iets lastiger uit om te doen in WebGL en heeft alleen echt nut als de doel computer veel VRAM heeft.

In elk geval stel ik voor om pas tegen het einde naar dit soort optimalisaties te kijken. De huidige implementatie is zo dat hij in principe redelijk makkelijk kan worden uitgebreid. Als we echt tegen problemen aan gaan lopen dan zijn er nog optimalisaties mogelijk, mits we een aantal compromieen sluiten.

Ook mooi om te horen dat m'n Type Script er fatsoenlijk uit ziet :)

@RoanH RoanH mentioned this pull request Apr 29, 2018
color[0], color[1], color[2], color[3],
color[0], color[1], color[2], color[3]];
this.gl.bufferData(this.gl.ARRAY_BUFFER, new Float32Array(colors), this.gl.STATIC_DRAW);
this.arrays.push({
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ik vraag me (voor de toekomst, evt optimalisatie) af of het niet efficiënter (en uberhaupt mogelijk / wenselijk) is om AL onze triangles/quads in één buffer te knallen die we maar één keertje hoeven te pushen.

Voor nu werkt dit allemaal prima.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Kleine buffers zijn efficienter. Dit is omdat je dan telkens al kleine taken naar de GPU stuurt. Dus de CPU en GPU zijn beiden tegelijk bezig. Anders dan doet eerste de CPU zijn werk en daarna de GPU pas. Ook kan het zijn dat een grote buffer niet in VRAM past. https://gamedev.stackexchange.com/questions/47550/why-is-it-faster-to-draw-lots-of-small-arrays-than-one-big-array
Het kan zijn dat we door onze erg specefieke use case wel optimalisaties kunnen maken hier in hoor, maar daar zouden we onderzoek naar moeten doen.

Copy link
Collaborator Author

@RoanH RoanH Apr 29, 2018

Choose a reason for hiding this comment

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

Los daar van is een werkende mega buffer maken en goed organiseren best lastig en zal redelijk wat tijd kosten om goed te implementeren. Althans dat denk ik (want ik had het al uitgeprobeerd), ook hebben we meer vertices nodig. Dus deze buffer zal ook groter zijn als de kleine bij elkaar opgeteld. Alhoewel er met heel erg slim vertex management misschien geen extra vertices nodig zijn. Moeten we tegen de tijd dat dit relevant is maar naar kijken.

//the model view matrix will later be used for user interaction
var modelviewMatrix = this.createMatrix();

this.gl.useProgram(this.shader);
Copy link
Collaborator

@NicoKNL NicoKNL Apr 29, 2018

Choose a reason for hiding this comment

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

this.gl gebruikt al this.shader als program. Lijkt mij meer iets voor direct na lijn 154 (this.initShaders();).

Mogelijk afhankelijk overigens van hoe we extra visualisaties gebruik laten maken van het canvas. (Canvas per visualisate, of shared? Vragen voor bij een volgende meeting / sprint review.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Klopt, maar deze call is relatief licht en zorgt er voor dat we later geen problemen krijgen als we eventueel meer shaders implementeren of als de OpenGL context ook voor andere dingen gebruikt word. Ik denk dat je er gelijk in hebt dat we de call kunnen verplaatsen, maar ik denk niet dat het een performance improvement geeft.

NicoKNL
NicoKNL previously approved these changes Apr 29, 2018
Copy link
Collaborator

@NicoKNL NicoKNL left a comment

Choose a reason for hiding this comment

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

Hoewel ik niet alle kleine details van de implementatie begrijp ziet dit er uit als een uitstekende start voor de OpenGL implementatie.

Paar comments geplaatst voor future consideration, verder wat mij betreft goedgekeurd.

@RoanH
Copy link
Collaborator Author

RoanH commented Apr 30, 2018

Don't merge this yet, still refactoring the code.

@RoanH
Copy link
Collaborator Author

RoanH commented Apr 30, 2018

Ok, ik denk dat dit zo goed is.

Het enige wat er qua logic veranderd is, is dat er nu een functie is om alle OpenGL buffers vrij te geven.

Verder is alles nu verdeeld over 2 (3 als je het window component mee telt) classes.

  • Matrix: voor alle matrix en vector operaties.
  • OpenGL: class met bijna alle logic, een instantie van deze class kan worden aangemaakt van een WebGL context. Dit maakt het nu ook veel makkelijker om verschillende OpenGL instanties te gebruiken op verschillende plaatsen / canvassen. Ook is shader management nu veel flexibeler.

Interfaces:

  • Shader: collectie van alle shader properties zodat we deze makkelijk door kunnen geven aan een OpenGL instantie.
  • Element: niets veranderd.

Alle OpenGL code bevind zich nu in ./opengl

Verder is het window component zelf nu heel clean. Er is een initialisatie subroutine voor OpenGL maar, verder ligt de focus nu op het berekenen van de visualisaties.

Uw review graag: @S4K4YUME @bartwesselink

@RoanH RoanH requested a review from NicoKNL April 30, 2018 12:19
//draw an axis aligned quad
private drawQuad(x: number, y: number, width: number, height: number, color: number[]): void {
//scale to coordinate space
x /= 800;
Copy link
Owner

Choose a reason for hiding this comment

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

Kunnen we deze waarden globaal definiëren? Dus ergens globaal opslaan 1600x900 als class-waarde, zodat deze niet 'uit de lucht' komen vallen.

Copy link
Collaborator Author

@RoanH RoanH Apr 30, 2018

Choose a reason for hiding this comment

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

Dat is raar, ik weet bijna zeker dat ik die globaal gedefinieert had, ga ik even naar kijken.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh, dit is een opmerking op een oude versie van de code. In de huidige versie zijn dit geen magic constants.

}

//initialises the shaders
private initShaders(): void {
Copy link
Owner

Choose a reason for hiding this comment

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

Is het een idee om deze functies in een OpenGL-util class te zetten? Met static-functies, waarbij de context als parameter meegegeven wordt.

Copy link
Collaborator Author

@RoanH RoanH Apr 30, 2018

Choose a reason for hiding this comment

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

Ik zag trouwens ook net dat deze opmerking over outdated code gaat dus ik weet niet of het nog relevant is:

Ik had deze functie eerst static gemaakt ja. Maar als je de class echt gaat gebruiken heb je altijd een OpenGL instantie bij de hand. De enige rede dat deze functie überhaupt een Shader returned en niet gewoon meteen de shader ook toewijst aan het OpenGL object is omdat we dan later meer shaders op een context kunnen gebruiken. Verder is de OpenGL class eigenlijk de util class, en het grote voordeel van een object over een static class in dit geval is dat je juist niet de webgl context als argument hoeft mee te geven, dus je hebt maar een object nodig waar je verder alles mee kunt doen.

private readonly HEIGHT: number;

constructor(gl: WebGLRenderingContext, width: number, height: number){
this.WIDTH = width;
Copy link
Owner

Choose a reason for hiding this comment

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

Dit kan denk ik niet. De property is readonly (komt in de build naar voren, meestal niet met ng serve). Dan is alleen de vraag of mijn opmerking nog geldt, omdat je dan geen readonly waarde hebt, waardoor inderdaad de berekeningen misschien trager zijn. Wat is jouw mening hierover? @RoanH

Copy link
Collaborator Author

@RoanH RoanH Apr 30, 2018

Choose a reason for hiding this comment

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

Als dit net zo werkt als Java (weet ik niet zeker, maar had ik wel aangenomen) dan word een property pas echt readonly na de constructor en kan hij een keer verandert worden in de constructor. Door van de constante een variable te maken zijn we eventuele compiler optimalisaties sowieso al kwijt. De enige manier waarop we die eventueel zouden kunnen behouden is als we de viewport resolution vast zetten op 1600x900. Maar dat geld dan voor alle instanties van de OpenGL class. Persoonlijk heb daar geen problemen mee, dus dat kunnen we doen.

Copy link
Owner

Choose a reason for hiding this comment

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

Opzich vind ik dat prima. Is inderdaad niet de mooiste, maar voor ons nu wel de beste denk ik. Verder is je PR goed 👍

Copy link
Collaborator Author

@RoanH RoanH Apr 30, 2018

Choose a reason for hiding this comment

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

Prima, dan fixeer ik de viewport.

@RoanH
Copy link
Collaborator Author

RoanH commented Apr 30, 2018

Ok, ik denk dat dit dan verder alles is. Als er nog opmerkingen zijn hoor ik het wel.

@bartwesselink bartwesselink merged commit 846fd3b into develop Apr 30, 2018
@RoanH RoanH mentioned this pull request Apr 30, 2018
@RoanH RoanH moved this from review to done in SPRINT 01 Apr 30, 2018
@RoanH RoanH deleted the feature/opengl branch May 2, 2018 11:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
SPRINT 01
  
done
Development

Successfully merging this pull request may close these issues.

Init webgl layer
3 participants