Merge pull request #755 from atom-minimap/reuse-DOMStylesReader

Author: aminya

lib/dom-styles-reader.js

@@ -21,6 +21,9 @@ export default class DOMStylesReader {
      */
     this.dummyNode = undefined
 
+    // used to check if the dummyNode is on the current targetNode
+    this.targetNode = undefined
+
     /**
      * Set to true once tokenized
      * @access private
@@ -40,16 +43,17 @@ export default class DOMStylesReader {
    *                                to build
    * @param  {string} property the name of the style property to compute
    * @param  {Node} targetNode
-   * @param  {boolean} cache whether to cache the computed value or not
+   * @param  {boolean} getFromCache whether to cache the computed value or not
    * @return {string} the computed property's value
    * used in CanvasDrawer
    */
-  retrieveStyleFromDom (scopes, property, targetNode, cache) {
+  retrieveStyleFromDom (scopes, property, targetNode, getFromCache) {
+    if (!scopes.length) { return '' } // no scopes
     const key = scopes.join(' ')
     let cachedData = this.domStylesCache.get(key)
 
     if (cachedData !== undefined) {
-      if (cache) { // if should get the value from the cache
+      if (getFromCache) { // if should get the value from the cache
         const value = cachedData[property]
         if (value !== undefined) {
           // value exists
@@ -59,7 +63,6 @@ export default class DOMStylesReader {
     } else {
       // key did not exist. create it
       cachedData = {}
-      this.domStylesCache.set(key, cachedData)
     }
 
     this.ensureDummyNodeExistence(targetNode)
@@ -99,12 +102,13 @@ export default class DOMStylesReader {
    * @access private
    */
   ensureDummyNodeExistence (targetNode) {
-    if (this.dummyNode === undefined) {
+    if (this.targetNode !== targetNode || this.dummyNode === undefined) {
       this.dummyNode = document.createElement('span')
       this.dummyNode.style.visibility = 'hidden'
 
       // attach to the target node
       targetNode.appendChild(this.dummyNode)
+      this.targetNode = targetNode
     }
   }
 

lib/main.js

@@ -6,6 +6,7 @@ import Minimap from './minimap'
 import config from './config.json'
 import * as PluginManagement from './plugin-management'
 import { treeSitterWarning } from './performance-monitor'
+import DOMStylesReader from './dom-styles-reader'
 
 export { default as config } from './config.json'
 export * from './plugin-management'
@@ -63,6 +64,12 @@ let subscriptionsOfCommands = null
      */
 export const emitter = new Emitter()
 
+
+/**
+  DOMStylesReader cache used for storing token colors
+*/
+let DOMStylesReaderInstance = null
+
 /**
    * Activates the minimap package.
    */
@@ -85,6 +92,8 @@ export function activate () {
   })
 
   editorsMinimaps = new Map()
+  DOMStylesReaderInstance = new DOMStylesReader()
+
   subscriptions = new CompositeDisposable()
   active = true
 
@@ -100,6 +109,8 @@ export function activate () {
 export function minimapViewProvider (model) {
   if (model instanceof Minimap) {
     const element = new MinimapElement()
+    // add DOMStylesReaderInstance
+    element.DOMStylesReader = DOMStylesReaderInstance;
     element.setModel(model)
     return element
   }
@@ -125,6 +136,7 @@ export function deactivate () {
   subscriptionsOfCommands.dispose()
   subscriptionsOfCommands = null
   editorsMinimaps = undefined
+  DOMStylesReaderInstance.invalidateDOMStylesCache()
   toggled = false
   active = false
 }
@@ -153,6 +165,7 @@ export function toggle () {
     toggled = true
     initSubscriptions()
   }
+  DOMStylesReaderInstance.invalidateDOMStylesCache()
 }
 
 /**
@@ -358,6 +371,11 @@ function initSubscriptions () {
       emitter.emit('did-create-minimap', minimap)
       minimapElement.attach()
     }),
+    // empty color cache if the theme changes
+    atom.themes.onDidChangeActiveThemes(() => {
+      DOMStylesReaderInstance.invalidateDOMStylesCache()
+      editorsMinimaps.forEach((minimap) => { atom.views.getView(minimap).requestForcedUpdate() })
+    }),
     treeSitterWarning()
   )
 }

lib/minimap-element.js

@@ -377,18 +377,6 @@ class MinimapElement {
     }
 
     this.subscriptions.add(
-
-      /*
-        We use `atom.styles.onDidAddStyleElement` instead of
-        `atom.themes.onDidChangeActiveThemes`.
-        Why? Currently, The style element will be removed first, and then re-added
-        and the `change` event has not be triggered in the process.
-      */
-      atom.styles.onDidAddStyleElement(() => {
-        this.DOMStylesReader.invalidateDOMStylesCache()
-        this.requestForcedUpdate()
-      }),
-
       this.subscribeToMediaQuery()
     )
   }

lib/mixins/canvas-drawer.js

@@ -5,7 +5,6 @@ import Mixin from 'mixto'
 
 import * as Main from '../main'
 import CanvasLayer from '../canvas-layer'
-import DOMStylesReader from '../dom-styles-reader'
 
 const SPEC_MODE = atom.inSpecMode()
 
@@ -76,11 +75,6 @@ export default class CanvasDrawer extends Mixin {
 
     // the maximum number of tokens to render in one line
     this.maxTokensInOneLine = atom.config.get('minimap.maxTokensInOneLine')
-
-    /**
-     * This MinimapElement's DOMStylesReader
-     */
-    this.DOMStylesReader = new DOMStylesReader()
   }
 
   /**
@@ -695,7 +689,7 @@ function drawLines (firstRow, lastRow, offsetRow, lineHeight, charHeight, charWi
     for (let iToken = 0; iToken < numTokenToRender; iToken++) {
       const token = editorTokensForScreenRow[iToken]
       const tokenText = token.text.replace(invisibleRegExp, ' ')
-      const tokenScopes = token.scopeDescriptor || token.scopes
+      const tokenScopes = token.scopes
 
       if (lastLine !== line) {
         x = 0

spec/minimap-element-spec.js

@@ -837,7 +837,7 @@ describe('MinimapElement', () => {
           // wait until all animations run out
           waitsFor(() => {
             nextAnimationFrame !== noAnimationFrame && nextAnimationFrame()
-            return editorElement.getScrollTop() >= 480
+            return editorElement.getScrollTop() >= 470 // flaky
           })
         })
 
@@ -870,7 +870,7 @@ describe('MinimapElement', () => {
             // wait until all animations run out
             waitsFor(() => {
               nextAnimationFrame !== noAnimationFrame && nextAnimationFrame()
-              return editorElement.getScrollTop() >= 480
+              return editorElement.getScrollTop() >= 470 //flaky
             })
           })
 
@@ -1198,9 +1198,7 @@ describe('MinimapElement', () => {
           spyOn(minimapElement, 'requestForcedUpdate').andCallThrough()
           spyOn(minimapElement.DOMStylesReader, 'invalidateDOMStylesCache').andCallThrough()
 
-          const styleNode = document.createElement('style')
-          styleNode.textContent = 'body{ color: #233 }'
-          atom.styles.emitter.emit('did-add-style-element', styleNode)
+          atom.themes.emitter.emit('did-change-active-themes')
         })
 
         waitsFor('minimap frame requested', () => {