Bug 34391 - Web Inspector: Introduce DivBasedTextViewer.
Summary: Web Inspector: Introduce DivBasedTextViewer.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Pavel Feldman
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-01-31 03:30 PST by Pavel Feldman
Modified: 2010-01-31 08:37 PST (History)
4 users (show)

See Also:


Attachments
[PATCH] Proposed change. (29.89 KB, patch)
2010-01-31 03:40 PST, Pavel Feldman
no flags Details | Formatted Diff | Diff
[PATCH] Same with less typos. (29.89 KB, patch)
2010-01-31 03:42 PST, Pavel Feldman
timothy: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Pavel Feldman 2010-01-31 03:30:33 PST
I am trying out a hybrid approach in source rendering. I thought it would be slow, but given that we do no real editing yet, it seems to work just fine. It will anyway be dead slow on debugging gmail scripts (where we create XXK line divs), but other than that it should be Ok.

Some highlights:
- It inherits from TextEditor and conforms to its API
- It re-uses TextEditor's code (and canvas) to render decorations such as program counter, breakpoints and messages
- It uses same TextEditorModel and highlighter
- It uses same code for line number rendering

The difference is that DivBasedTextViewer creates a div-per line and instantly adds all of them to the editor's sheet. This creates a non-highlighted text that has proper scrollers, browser-based selection, etc. We maintain original scroller handlers and hence when user scrolls to a dirty region, paintLines is being called. Instead of painting lines on the canvas, this viewer replaces them with their highlighted versions using same highlighter logic. As a result, when user opens a source view, he gets a nice highlighted text (highlight is only created for visible area). Then if he scrolls to the middle of the large file, highlighter creates markup attributes on the text model in the background (same control flow as canvas-based). When highlighting data for the visible area is ready, it gets 'repainted', i.e. replaced with highlighted spans.

Seems to be much more efficient than what we've had before and allows us to evolve both versions (canvas and span-based) in parallel.
Comment 1 Pavel Feldman 2010-01-31 03:40:03 PST
Created attachment 47789 [details]
[PATCH] Proposed change.
Comment 2 Pavel Feldman 2010-01-31 03:42:19 PST
Created attachment 47790 [details]
[PATCH] Same with less typos.
Comment 3 Timothy Hatcher 2010-01-31 07:33:59 PST
This approch is exactly what I was thinking.
Comment 4 Timothy Hatcher 2010-01-31 07:47:24 PST
Comment on attachment 47790 [details]
[PATCH] Same with less typos.

> +WebInspector.DivBasedTextViewer = function(textModel, platform)
> +{
> +    WebInspector.TextEditor.call(this, textModel, platform);

I'm not fond of "DivBasedTextViewer" as a class name. What about "NativeTextViewer", "SimpleTextViewer" or "ClassicTextViewer"? Or, not great but, "DOMBasedTextViewer".

> +    useCanvasBasedEditor: true

Should be false?
Comment 5 Pavel Feldman 2010-01-31 07:51:09 PST
(In reply to comment #4)
> (From update of attachment 47790 [details])
> > +WebInspector.DivBasedTextViewer = function(textModel, platform)
> > +{
> > +    WebInspector.TextEditor.call(this, textModel, platform);
> 
> I'm not fond of "DivBasedTextViewer" as a class name. What about
> "NativeTextViewer", "SimpleTextViewer" or "ClassicTextViewer"? Or, not great
> but, "DOMBasedTextViewer".
> 

Lets try NativeTextViewer.

> > +    useCanvasBasedEditor: true
> 
> Should be false?

Not until I fix todos mentioned in the changelog. But that should not take long.
Comment 6 Timothy Hatcher 2010-01-31 07:52:45 PST
(In reply to comment #0)
> I am trying out a hybrid approach in source rendering. I thought it would be
> slow, but given that we do no real editing yet, it seems to work just fine. It
> will anyway be dead slow on debugging gmail scripts (where we create XXK line
> divs), but other than that it should be Ok.

Couldn't we also avoid appending all the line divs and insert spacers until those lines need painted?

I justt things like select all would not work then.

That reminds me, I think you need to add the "-webkit-user-select: text" style to the top level element for the viewer.
Comment 7 Timothy Hatcher 2010-01-31 07:53:19 PST
"I guess things like select all would not work then."
Comment 8 Pavel Feldman 2010-01-31 08:37:21 PST
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebCore/ChangeLog
	M	WebCore/WebCore.gypi
	M	WebCore/WebCore.vcproj/WebCore.vcproj
	A	WebCore/inspector/front-end/NativeTextViewer.js
	M	WebCore/inspector/front-end/Settings.js
	M	WebCore/inspector/front-end/SourceFrame.js
	M	WebCore/inspector/front-end/TextEditor.js
	M	WebCore/inspector/front-end/TextEditorHighlighter.js
	M	WebCore/inspector/front-end/WebKit.qrc
	M	WebCore/inspector/front-end/inspector.css
	M	WebCore/inspector/front-end/inspector.html
	M	WebCore/inspector/front-end/textEditor.css
Committed r54110