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.

Sign up for GitHub

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

Jump to bottom

Patch #1: VRKit, ModuleManager, PostProcessor #252

Merged
merged 9 commits into from
Jun 17, 2017
Merged

Patch #1: VRKit, ModuleManager, PostProcessor #252

merged 9 commits into from
Jun 17, 2017

Conversation

sasha240100
Copy link
Member

@sasha240100 sasha240100 commented Jun 16, 2017 •
edited
Loading

  • VRKit
    • VRModule
    • VRControls
  • ModuleManager
    • Use redux
    • .define(), .use()
  • PostProcessor
    • Use postprocessing npm module
    • Simpler API
      • .shader(), .pass()
      • other

API changes

  • ModuleManager
    • Added manager.define("name"). Adds module to modules list with this name. Is optional for modules without much API.
    • manager.get('renderer', true) -> manager.use('rendering')
  • PostProcessor
    • postprocessor.shader(material) - Makes ShaderPass from material automatically and uses it.
    • postprocessor.pass(pass) - Adds a pass.
    • postprocessor.renderToScreen() - Make this pass be rendered to screen.
    • postprocessor.name(name) - Name current pass.
    • postprocessor.to(name) - Change current pass.

Examples added

  • VRKit
    • VRKit/basic
    • VRKit/controls
    • VRKit/postprocessing (work in progress)

@sasha240100 sasha240100 self-assigned this Jun 16, 2017
hirako2000
hirako2000 reviewed Jun 16, 2017
View reviewed changes
Copy link
Collaborator

@hirako2000 hirako2000 left a comment

Choose a reason for hiding this comment

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

no unit test?

modules/whs-vrkit/package.json Outdated
"name": "whs-vrkit",
"main": "build/VRKit.js",
"scripts": {
"build": "webpack",
Copy link
Collaborator

Choose a reason for hiding this comment

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

this won't work unless webpack is installed globally.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's how we do it

modules/whs-vrkit/README.md Outdated
const app = new WHS.App([
new WHS.ElementModule(), // This module is required
// other modules
new StatsModule(StatsModule.codes.fps) // or just "0"
Copy link
Collaborator

Choose a reason for hiding this comment

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

? is that not VRKit?

modules/whs-vrkit/README.md Outdated

## Usage

```javascript
const app = new WHS.App([
new WHS.ElementModule(), // This module is required
// other modules
new StatsModule(StatsModule.codes.fps) // or just "0"
new VRKit.VRModule() // enables VR
Copy link
Collaborator

Choose a reason for hiding this comment

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

OK ignore my other comment.

examples/VRKit/post-processing/script.js
new WHS.ResizeModule(),
new StatsModule(),
new VRKit.VRModule(),
// postprocessor
Copy link
Collaborator

Choose a reason for hiding this comment

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

let's remove commented out code.

Copy link
Member Author

Choose a reason for hiding this comment

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

@hirako2000 It will be used when we'll make post-processing work with VREffect.

examples/VRKit/post-processing/script.js Outdated
// // .name('MyShader')
// .renderToScreen();

const sphere = new WHS.Sphere({ // Create sphere comonent.
Copy link
Collaborator

Choose a reason for hiding this comment

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

typo, and no need for a comment imo

modules/whs-vrkit/src/lib/VRControls.js Outdated
@@ -0,0 +1,149 @@
/**
Copy link
Collaborator

Choose a reason for hiding this comment

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

looks like this lib is on npm. maybe use it as a dep instead?
https://github.com/halvves/three-vrcontrols-module

src/modules/app/RenderingModule.js Outdated
}

play() {
this.renderLoop.start();
// this.renderLoop.start();
Copy link
Collaborator

Choose a reason for hiding this comment

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

commented out code.

src/modules/app/index.js Outdated
@@ -3,7 +3,7 @@ export * from './CameraModule';
export * from './RenderingModule';
export * from './SceneModule';
export * from './ResizeModule';
export * from './PostProcessorModule';
Copy link
Collaborator

Choose a reason for hiding this comment

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

that would break the API

modules/whs-vrkit/src/lib/WebVR.js
@@ -0,0 +1,186 @@
/**
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should publish code like this if not already on npm yet.

Copy link
Member Author

Choose a reason for hiding this comment

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

@hirako2000 No need. @mrdoob says VR API in Three.js will be changed soon. No more even VREffect & VRControls

modules/whs-vrkit/webpack.config.babel.js
);

export default {
devtool: isProduction ? '#hidden-source-map' : '#source-map',
Copy link
Collaborator

Choose a reason for hiding this comment

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

is that correct?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I saw it somewhere:)

@sasha240100 sasha240100 merged commit c72087b into beta Jun 17, 2017
@sasha240100 sasha240100 changed the title VRKit, ModuleManager, PostProcessor patch Patch #1: VRKit, ModuleManager, PostProcessor Jun 17, 2017
@sasha240100 sasha240100 deleted the whs-vrkit branch September 7, 2017 21:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hirako2000 hirako2000 hirako2000 left review comments

Assignees

@sasha240100 sasha240100

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@sasha240100 @hirako2000