Fix the maximum nodes limitation in Tilt

RESOLVED FIXED in Firefox 13

Status

()

Firefox
Developer Tools: Inspector
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: vporof, Assigned: vporof)

Tracking

12 Branch
Firefox 13
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [tilt])

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
If a webpage has more than ~5000 nodes, it will be chopped off in Tilt, in a quite ugly fashion. This shouldn't happen.
(Assignee)

Updated

5 years ago
Assignee: nobody → vporof
Status: NEW → ASSIGNED
(Assignee)

Updated

5 years ago
Whiteboard: [tilt]
(Assignee)

Comment 1

5 years ago
Created attachment 597058 [details] [diff] [review]
v1
Attachment #597058 - Flags: review?(rcampbell)
(Assignee)

Updated

5 years ago
Depends on: 723937
(Assignee)

Comment 2

5 years ago
Try: https://tbpl.mozilla.org/?tree=Try&rev=47af48d6e93e
(Assignee)

Updated

5 years ago
Blocks: 725717
Comment on attachment 597058 [details] [diff] [review]
v1

+// index buffer of 12 unsigned int elements, obviously one for each vertex;

"obviously"

+// if a webpage has enough nodes to overflow the index buffer elements size,

+const MAX_GROUP_NODES = Math.pow(2, Uint16Array.BYTES_PER_ELEMENT * 8) / 12 - 1;

when does a Uint16Array have something other than 2 bytes per element? This feels like we could hardcode the value.

Also, this returns a non-integer value of 5460.333333333333. That seems... imprecise.

+    for (let i = 0, len = mesh.length; i < len; i++) {

presumably, the number of mesh groups is going to be 1 in most cases?

I'm gonna shelf this until my brain can process the rest of it.
(Assignee)

Comment 4

5 years ago
(In reply to Rob Campbell [:rc] (robcee) from comment #3)
> Comment on attachment 597058 [details] [diff] [review]
> v1
> 
> +const MAX_GROUP_NODES = Math.pow(2, Uint16Array.BYTES_PER_ELEMENT * 8) / 12
> - 1;
> 
> when does a Uint16Array have something other than 2 bytes per element? This
> feels like we could hardcode the value.
> 

I don't know.. on quantum computers? On Darth Vader's helmet? :)
Why hardcode the value if there's a logical formula to get it?

> Also, this returns a non-integer value of 5460.333333333333. That seems...
> imprecise.
> 

Doesn't affect anything. I can parseInt if you wish.

> +    for (let i = 0, len = mesh.length; i < len; i++) {
> 
> presumably, the number of mesh groups is going to be 1 in most cases?
> 

Yup.
(In reply to Victor Porof from comment #4)
> (In reply to Rob Campbell [:rc] (robcee) from comment #3)
> > Comment on attachment 597058 [details] [diff] [review]
> > v1
> > 
> > +const MAX_GROUP_NODES = Math.pow(2, Uint16Array.BYTES_PER_ELEMENT * 8) / 12
> > - 1;
> > 
> > when does a Uint16Array have something other than 2 bytes per element? This
> > feels like we could hardcode the value.
> > 
> 
> I don't know.. on quantum computers? On Darth Vader's helmet? :)

lol, imo.

> Why hardcode the value if there's a logical formula to get it?

Ok, fair enough.

> > Also, this returns a non-integer value of 5460.333333333333. That seems...
> > imprecise.
> > 
> 
> Doesn't affect anything. I can parseInt if you wish.

maybe a Math.floor()?

> > +    for (let i = 0, len = mesh.length; i < len; i++) {
> > 
> > presumably, the number of mesh groups is going to be 1 in most cases?
> 
> Yup.

OK.
Attachment #597058 - Flags: review?(rcampbell) → review+
(Assignee)

Updated

5 years ago
Whiteboard: [tilt] → [tilt][land-in-fx-team]
https://hg.mozilla.org/integration/fx-team/rev/80e183c9caff
Whiteboard: [tilt][land-in-fx-team] → [tilt][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/80e183c9caff
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Whiteboard: [tilt][fixed-in-fx-team] → [tilt]
Target Milestone: --- → Firefox 13
You need to log in before you can comment on or make changes to this bug.