Closed Bug 734038 Opened 12 years ago Closed 8 years ago

Refactor Tilt to distinguish between model and view matrices


(Firefox Graveyard :: Developer Tools: 3D View, defect, P2)

12 Branch


(Not tracked)



(Reporter: vporof, Unassigned)




(1 file, 2 obsolete files)

Currently we only have model-view, which is the product of the two. The common practice when having cameras is to distinguish between the two matrices, have "model" for the world transforms and "view" for the camera lookAt matrix. This would be of great help for bug 721085.
Blocks: 721085
Assignee: nobody → vporof
Whiteboard: [tilt]
Attached patch v1 (obsolete) — Splinter Review
WIP. Adding here for safekeeping. Rather huge patch.
Attached patch v2 (obsolete) — Splinter Review
Does the refactoring. I can't be 100% be sure this patch will remain unchanged after I finish bug 721085, but I can't think of any obvious reason why I'd need anything else.

Tests fail. I think it makes most sense to deal with them in 721085, since we don't have an arcball now :)

A few helper notes, since this may be a bit overwhelming:

* patch line 19: This bugged me for a while. I don't like writing this.chromeWindow.InspectorUI.INSPECTOR_NOTIFICATIONS every time, so I wrote a getters for both Tilt and Inspector notifications

* patch lines 36-42: We don't have an arcball, zooming shouldn't depend on anything so I moved this in the presenter as a resetDocumentZoom() func

* patch line 161: I really don't know why we didn't have contextFlags in the constructor till now. It's a bad idea not to have them, while create3DContext() does.

* patch line 258 (and similar stuff): Drawing is now made on a material basis, which changes states without knowing the existing context. An if is faster than gl.someFunction

* patch line 327: Because we don't have model and view matrices embedded in the renderer itself, making it able to draw custom primitives is stupid. That's the reason for the custom camera+mesh+material approach you'll see in the patch. This is why this color shader was moved in TiltVisualization, alongside the mesh shader.

What follows from here is pretty much hg not being able to understand what happened. Don't follow the + vs - in relation to each other, it's all a big chunk of deleted stuff, followed by the camera + mesh + material implementation.

* patch lines 1164, 1220 (and a couple similar): This stuff wasn't used anywhere, and it's a leftover from the addon. I removed it.

* patch line 1559: This function used to be in the renderer. This was dumb, it's a matrix function not depending on anything, so it should be in TiltMath along with the other matrix functions

* patch line 2811: This is exactly what will be reimplemented in 721085. Because of the current camera + mesh approach, it will be a much, much smaller constructor.

* patch line 3587: The transparency was hardcoded in js. Bad idea, and since we removed the possibility of drawing custom primitives in the renderer (responsable with drawing the highlight quad), I thought it's a good idea to fix this here as well.

Apart from that, there are a couple comment and typo fixes here and there.
Ping me if I forgot anything.
Attachment #609514 - Attachment is obsolete: true
Attachment #610037 - Flags: feedback?(rcampbell)
Attached patch v3Splinter Review
Played with this a bit during the weekend.
Attachment #610037 - Attachment is obsolete: true
Attachment #611414 - Flags: feedback?(rcampbell)
Attachment #610037 - Flags: feedback?(rcampbell)
Blocks: 768392
Priority: -- → P2
New component triage. Filter on HORSE MASKS.
Component: Developer Tools: Inspector → Developer Tools: 3D View
Whiteboard: [tilt]
Attachment #611414 - Flags: feedback?(rcampbell)
Assignee: vporof → nobody
The 3D View has been removed in Firefox 47. Closing all bugs inside that component as the component will be moved to Firefox Graveyard.

Closed: 8 years ago
Resolution: --- → WONTFIX
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.