Conversation
…button in the model
…g and disabling each component
msfeldstein
left a comment
msfeldstein
left a comment
There was a problem hiding this comment.
Still work to do before committing but i wanted to get feedback on my direction.
| if-no-vr-headset="visible: false" | ||
| paint-controls="hand: right" | ||
| screenshot-camera | ||
| trigger-mode="brush" |
There was a problem hiding this comment.
Perhaps this should be called action-mode since it might support more than just the trigger button, IE using the grips for scaling instead of undo
| }, | ||
|
|
||
| triggerModeChanged: function() { | ||
| this.enabled = this.data.enabled && this.el.getAttribute('trigger-mode') == 'brush' |
There was a problem hiding this comment.
Is the proper way to do this to have another component that manages the trigger-mode attribute to data.enabled binding? To make this more reusable it would be nice if this didn't have to worry about the external trigger-mode attribute
| var _gl = renderer.context | ||
| var framebuffer = renderer.properties.get(this.renderTarget).__webglFramebuffer | ||
| _gl.bindFramebuffer( _gl.FRAMEBUFFER, framebuffer ); | ||
| _gl.readPixels( 0, 0, width, height, _gl.RGBA,_gl.UNSIGNED_BYTE, this.pixels ); |
There was a problem hiding this comment.
I couldn't get WebGLRenderer.readRenderTargetPixels to work here, unsure why as this looks like the same code. Is there a good way to debug Threejs while working on a-painter?
| }, 'image/png'); | ||
| }, | ||
|
|
||
| flipPixelsVertically: function (pixels, width, height) { |
There was a problem hiding this comment.
This flipping seems to cause frame skipping. It actually kind of works because the white flash feels like taking a flash photograph, but it should probably be moved into a web worker or something.
src/components/ui.js
Outdated
| AFRAME.registerComponent('ui', { | ||
| schema: { brightness: { default: 1.0, max: 1.0, min: 0.0 } }, | ||
| dependencies: ['ui-raycaster'], | ||
| dependencies: ['ui-raycaster', 'screenshot-camera', 'brush'], |
There was a problem hiding this comment.
Is there a reason brush wasn't a dependency before?
There was a problem hiding this comment.
'brush' is a dependency of paint-controls. Any reason do you need it here as a dependency? I would do a new component screenshot-controls that it's enabled when clicking on the screenshot button. The ui component manages what controls are enable/disable. For instance: You click on screenshot button then you disable paint-controls and enable screenshot-controls
There was a problem hiding this comment.
i think i misunderstood dependencies. Is ui-raycaster needed as a dependency because we modify it in the init() method?
And are you thinking screenshot-controls would be different than the screenshot-camera component i added, or I should just rename it?
| toggleCaptureCamera: function() { | ||
| var enableCamera = this.handEl.getAttribute('trigger-mode') != 'camera' | ||
| this.handEl.setAttribute('trigger-mode', enableCamera ? 'camera' : 'brush') | ||
| this.handEl.emit('trigger-mode-changed') |
There was a problem hiding this comment.
Is there a built-in way to listen for any attribute changes, or do i need to emit custom events each time i change?
src/index.js
Outdated
| requestAnimationFrame(f) | ||
| AFRAME.TWEEN.update() | ||
| } | ||
| requestAnimationFrame(f) |
There was a problem hiding this comment.
This is just to fix the chrome animation frame timing bug, will remove before committing.
There was a problem hiding this comment.
You could develop on Firefox Nightly now and you don't need it and get better latency ;)
| } | ||
| case name === 'clear': { | ||
| if (!this.pressedObjects[name]) { | ||
| this.toggleCaptureCamera(); |
There was a problem hiding this comment.
Need to get another button in the menu for this. If people like the change maybe @feiss can hook up a camera button.
2 participants
This still has work that needs to be done but i wanted to get some feedback