Merge pull request #6143 from robintown/hide-events-perf

src/components/structures/MessagePanel.js

@@ -25,10 +25,11 @@ import * as sdk from '../../index';
 
 import {MatrixClientPeg} from '../../MatrixClientPeg';
 import SettingsStore from '../../settings/SettingsStore';
+import RoomContext from "../../contexts/RoomContext";
 import {Layout, LayoutPropType} from "../../settings/Layout";
 import {_t} from "../../languageHandler";
 import {haveTileForEvent} from "../views/rooms/EventTile";
-import {textForEvent} from "../../TextForEvent";
+import {hasText} from "../../TextForEvent";
 import IRCTimelineProfileResizer from "../views/elements/IRCTimelineProfileResizer";
 import DMRoomMap from "../../utils/DMRoomMap";
 import NewRoomIntro from "../views/rooms/NewRoomIntro";
@@ -151,6 +152,8 @@ export default class MessagePanel extends React.Component {
         enableFlair: PropTypes.bool,
     };
 
+    static contextType = RoomContext;
+
     constructor(props) {
         super(props);
 
@@ -380,7 +383,7 @@ export default class MessagePanel extends React.Component {
         // Always show highlighted event
         if (this.props.highlightedEventId === mxEv.getId()) return true;
 
-        return !shouldHideEvent(mxEv);
+        return !shouldHideEvent(mxEv, this.context);
     }
 
     _readMarkerForEvent(eventId, isLastEvent) {
@@ -1164,11 +1167,8 @@ class MemberGrouper {
 
     add(ev) {
         if (ev.getType() === 'm.room.member') {
-            // We'll just double check that it's worth our time to do so, through an
-            // ugly hack. If textForEvent returns something, we should group it for
-            // rendering but if it doesn't then we'll exclude it.
-            const renderText = textForEvent(ev);
-            if (!renderText || renderText.trim().length === 0) return; // quietly ignore
+            // We can ignore any events that don't actually have a message to display
+            if (!hasText(ev)) return;
         }
         this.readMarker = this.readMarker || this.panel._readMarkerForEvent(
             ev.getId(),

src/components/structures/RoomView.tsx

@@ -153,7 +153,6 @@ export interface IState {
     canPeek: boolean;
     showApps: boolean;
     isPeeking: boolean;
-    showReadReceipts: boolean;
     showRightPanel: boolean;
     // error object, as from the matrix client/server API
     // If we failed to load information about the room,
@@ -181,6 +180,12 @@ export interface IState {
     canReact: boolean;
     canReply: boolean;
     layout: Layout;
+    lowBandwidth: boolean;
+    showReadReceipts: boolean;
+    showRedactions: boolean;
+    showJoinLeaves: boolean;
+    showAvatarChanges: boolean;
+    showDisplaynameChanges: boolean;
     matrixClientIsReady: boolean;
     showUrlPreview?: boolean;
     e2eStatus?: E2EStatus;
@@ -198,8 +203,7 @@ export default class RoomView extends React.Component<IProps, IState> {
     private readonly dispatcherRef: string;
     private readonly roomStoreToken: EventSubscription;
     private readonly rightPanelStoreToken: EventSubscription;
-    private readonly showReadReceiptsWatchRef: string;
-    private readonly layoutWatcherRef: string;
+    private settingWatchers: string[];
 
     private unmounted = false;
     private permalinkCreators: Record<string, RoomPermalinkCreator> = {};
@@ -230,7 +234,6 @@ export default class RoomView extends React.Component<IProps, IState> {
             canPeek: false,
             showApps: false,
             isPeeking: false,
-            showReadReceipts: true,
             showRightPanel: RightPanelStore.getSharedInstance().isOpenForRoom,
             joining: false,
             atEndOfLiveTimeline: true,
@@ -240,6 +243,12 @@ export default class RoomView extends React.Component<IProps, IState> {
             canReact: false,
             canReply: false,
             layout: SettingsStore.getValue("layout"),
+            lowBandwidth: SettingsStore.getValue("lowBandwidth"),
+            showReadReceipts: true,
+            showRedactions: true,
+            showJoinLeaves: true,
+            showAvatarChanges: true,
+            showDisplaynameChanges: true,
             matrixClientIsReady: this.context && this.context.isInitialSyncComplete(),
             dragCounter: 0,
         };
@@ -266,9 +275,14 @@ export default class RoomView extends React.Component<IProps, IState> {
         WidgetEchoStore.on(UPDATE_EVENT, this.onWidgetEchoStoreUpdate);
         WidgetStore.instance.on(UPDATE_EVENT, this.onWidgetStoreUpdate);
 
-        this.showReadReceiptsWatchRef = SettingsStore.watchSetting("showReadReceipts", null,
-            this.onReadReceiptsChange);
-        this.layoutWatcherRef = SettingsStore.watchSetting("layout", null, this.onLayoutChange);
+        this.settingWatchers = [
+            SettingsStore.watchSetting("layout", null, () =>
+                this.setState({ layout: SettingsStore.getValue("layout") }),
+            ),
+            SettingsStore.watchSetting("lowBandwidth", null, () =>
+                this.setState({ lowBandwidth: SettingsStore.getValue("lowBandwidth") }),
+            ),
+        ];
     }
 
     private onWidgetStoreUpdate = () => {
@@ -324,9 +338,42 @@ export default class RoomView extends React.Component<IProps, IState> {
             // we should only peek once we have a ready client
             shouldPeek: this.state.matrixClientIsReady && RoomViewStore.shouldPeek(),
             showReadReceipts: SettingsStore.getValue("showReadReceipts", roomId),
+            showRedactions: SettingsStore.getValue("showRedactions", roomId),
+            showJoinLeaves: SettingsStore.getValue("showJoinLeaves", roomId),
+            showAvatarChanges: SettingsStore.getValue("showAvatarChanges", roomId),
+            showDisplaynameChanges: SettingsStore.getValue("showDisplaynameChanges", roomId),
             wasContextSwitch: RoomViewStore.getWasContextSwitch(),
         };
 
+        // Add watchers for each of the settings we just looked up
+        this.settingWatchers = this.settingWatchers.concat([
+            SettingsStore.watchSetting("showReadReceipts", null, () =>
+                this.setState({
+                    showReadReceipts: SettingsStore.getValue("showReadReceipts", roomId),
+                }),
+            ),
+            SettingsStore.watchSetting("showRedactions", null, () =>
+                this.setState({
+                    showRedactions: SettingsStore.getValue("showRedactions", roomId),
+                }),
+            ),
+            SettingsStore.watchSetting("showJoinLeaves", null, () =>
+                this.setState({
+                    showJoinLeaves: SettingsStore.getValue("showJoinLeaves", roomId),
+                }),
+            ),
+            SettingsStore.watchSetting("showAvatarChanges", null, () =>
+                this.setState({
+                    showAvatarChanges: SettingsStore.getValue("showAvatarChanges", roomId),
+                }),
+            ),
+            SettingsStore.watchSetting("showDisplaynameChanges", null, () =>
+                this.setState({
+                    showDisplaynameChanges: SettingsStore.getValue("showDisplaynameChanges", roomId),
+                }),
+            ),
+        ]);
+
         if (!initial && this.state.shouldPeek && !newState.shouldPeek) {
             // Stop peeking because we have joined this room now
             this.context.stopPeeking();
@@ -635,18 +682,16 @@ export default class RoomView extends React.Component<IProps, IState> {
             );
         }
 
-        if (this.showReadReceiptsWatchRef) {
-            SettingsStore.unwatchSetting(this.showReadReceiptsWatchRef);
-        }
-
         // cancel any pending calls to the rate_limited_funcs
         this.updateRoomMembers.cancelPendingCall();
 
         // no need to do this as Dir & Settings are now overlays. It just burnt CPU.
         // console.log("Tinter.tint from RoomView.unmount");
         // Tinter.tint(); // reset colourscheme
 
-        SettingsStore.unwatchSetting(this.layoutWatcherRef);
+        for (const watcher of this.settingWatchers) {
+            SettingsStore.unwatchSetting(watcher);
+        }
     }
 
     private onUserScroll = () => {
@@ -816,7 +861,7 @@ export default class RoomView extends React.Component<IProps, IState> {
             // update unread count when scrolled up
             if (!this.state.searchResults && this.state.atEndOfLiveTimeline) {
                 // no change
-            } else if (!shouldHideEvent(ev)) {
+            } else if (!shouldHideEvent(ev, this.state)) {
                 this.setState((state, props) => {
                     return {numUnreadMessages: state.numUnreadMessages + 1};
                 });

src/components/structures/TimelinePanel.js

@@ -26,6 +26,7 @@ import {EventTimeline} from "matrix-js-sdk/src/models/event-timeline";
 import {TimelineWindow} from "matrix-js-sdk/src/timeline-window";
 import { _t } from '../../languageHandler';
 import {MatrixClientPeg} from "../../MatrixClientPeg";
+import RoomContext from "../../contexts/RoomContext";
 import UserActivity from "../../UserActivity";
 import Modal from "../../Modal";
 import dis from "../../dispatcher/dispatcher";
@@ -125,6 +126,8 @@ class TimelinePanel extends React.Component {
         alwaysShowTimestamps: PropTypes.bool,
     }
 
+    static contextType = RoomContext;
+
     // a map from room id to read marker event timestamp
     static roomReadMarkerTsMap = {};
 
@@ -1288,7 +1291,7 @@ class TimelinePanel extends React.Component {
 
             const shouldIgnore = !!ev.status || // local echo
                 (ignoreOwn && ev.sender && ev.sender.userId == myUserId);   // own message
-            const isWithoutTile = !haveTileForEvent(ev) || shouldHideEvent(ev);
+            const isWithoutTile = !haveTileForEvent(ev) || shouldHideEvent(ev, this.context);
 
             if (isWithoutTile || !node) {
                 // don't start counting if the event should be ignored,

src/components/views/avatars/BaseAvatar.tsx

@@ -22,6 +22,7 @@ import classNames from 'classnames';
 import * as AvatarLogic from '../../../Avatar';
 import SettingsStore from "../../../settings/SettingsStore";
 import AccessibleButton from '../elements/AccessibleButton';
+import RoomContext from "../../../contexts/RoomContext";
 import MatrixClientContext from "../../../contexts/MatrixClientContext";
 import {useEventEmitter} from "../../../hooks/useEventEmitter";
 import {toPx} from "../../../utils/units";
@@ -44,12 +45,12 @@ interface IProps {
     className?: string;
 }
 
-const calculateUrls = (url, urls) => {
+const calculateUrls = (url, urls, lowBandwidth) => {
     // work out the full set of urls to try to load. This is formed like so:
     // imageUrls: [ props.url, ...props.urls ]
 
     let _urls = [];
-    if (!SettingsStore.getValue("lowBandwidth")) {
+    if (!lowBandwidth) {
         _urls = urls || [];
 
         if (url) {
@@ -63,15 +64,21 @@ const calculateUrls = (url, urls) => {
 };
 
 const useImageUrl = ({url, urls}): [string, () => void] => {
-    const [imageUrls, setUrls] = useState<string[]>(calculateUrls(url, urls));
+    // Since this is a hot code path and the settings store can be slow, we
+    // use the cached lowBandwidth value from the room context if it exists
+    const roomContext = useContext(RoomContext);
+    const lowBandwidth = roomContext ?
+        roomContext.lowBandwidth : SettingsStore.getValue("lowBandwidth");
+
+    const [imageUrls, setUrls] = useState<string[]>(calculateUrls(url, urls, lowBandwidth));
     const [urlsIndex, setIndex] = useState<number>(0);
 
     const onError = useCallback(() => {
         setIndex(i => i + 1); // try the next one
     }, []);
 
     useEffect(() => {
-        setUrls(calculateUrls(url, urls));
+        setUrls(calculateUrls(url, urls, lowBandwidth));
         setIndex(0);
     }, [url, JSON.stringify(urls)]); // eslint-disable-line react-hooks/exhaustive-deps
 

src/components/views/rooms/EventTile.tsx

@@ -25,7 +25,7 @@ import { RoomMember } from "matrix-js-sdk/src/models/room-member";
 
 import ReplyThread from "../elements/ReplyThread";
 import { _t } from '../../../languageHandler';
-import * as TextForEvent from "../../../TextForEvent";
+import { hasText } from "../../../TextForEvent";
 import * as sdk from "../../../index";
 import dis from '../../../dispatcher/dispatcher';
 import SettingsStore from "../../../settings/SettingsStore";
@@ -1210,7 +1210,7 @@ export function haveTileForEvent(e) {
     const handler = getHandlerTile(e);
     if (handler === undefined) return false;
     if (handler === 'messages.TextualEvent') {
-        return TextForEvent.textForEvent(e) !== '';
+        return hasText(e);
     } else if (handler === 'messages.RoomCreate') {
         return Boolean(e.getContent()['predecessor']);
     } else {

src/contexts/RoomContext.ts

@@ -31,7 +31,6 @@ const RoomContext = createContext<IState>({
     canPeek: false,
     showApps: false,
     isPeeking: false,
-    showReadReceipts: true,
     showRightPanel: true,
     joining: false,
     atEndOfLiveTimeline: true,
@@ -41,6 +40,12 @@ const RoomContext = createContext<IState>({
     canReact: false,
     canReply: false,
     layout: Layout.Group,
+    lowBandwidth: false,
+    showReadReceipts: true,
+    showRedactions: true,
+    showJoinLeaves: true,
+    showAvatarChanges: true,
+    showDisplaynameChanges: true,
     matrixClientIsReady: false,
     dragCounter: 0,
 });

src/i18n/strings/en_EN.json

@@ -556,8 +556,8 @@
     "%(senderName)s made future room history visible to all room members.": "%(senderName)s made future room history visible to all room members.",
     "%(senderName)s made future room history visible to anyone.": "%(senderName)s made future room history visible to anyone.",
     "%(senderName)s made future room history visible to unknown (%(visibility)s).": "%(senderName)s made future room history visible to unknown (%(visibility)s).",
-    "%(userId)s from %(fromPowerLevel)s to %(toPowerLevel)s": "%(userId)s from %(fromPowerLevel)s to %(toPowerLevel)s",
     "%(senderName)s changed the power level of %(powerLevelDiffText)s.": "%(senderName)s changed the power level of %(powerLevelDiffText)s.",
+    "%(userId)s from %(fromPowerLevel)s to %(toPowerLevel)s": "%(userId)s from %(fromPowerLevel)s to %(toPowerLevel)s",
     "%(senderName)s changed the pinned messages for the room.": "%(senderName)s changed the pinned messages for the room.",
     "%(widgetName)s widget modified by %(senderName)s": "%(widgetName)s widget modified by %(senderName)s",
     "%(widgetName)s widget added by %(senderName)s": "%(widgetName)s widget added by %(senderName)s",

src/shouldHideEvent.ts

@@ -17,6 +17,7 @@
 import {MatrixEvent} from "matrix-js-sdk/src/models/event";
 
 import SettingsStore from "./settings/SettingsStore";
+import {IState} from "./components/structures/RoomView";
 
 interface IDiff {
     isMemberEvent: boolean;
@@ -47,11 +48,18 @@ function memberEventDiff(ev: MatrixEvent): IDiff {
     return diff;
 }
 
-export default function shouldHideEvent(ev: MatrixEvent): boolean {
-    // Wrap getValue() for readability. Calling the SettingsStore can be
-    // fairly resource heavy, so the checks below should avoid hitting it
-    // where possible.
-    const isEnabled = (name) => SettingsStore.getValue(name, ev.getRoomId());
+/**
+ * Determines whether the given event should be hidden from timelines.
+ * @param ev The event
+ * @param ctx An optional RoomContext to pull cached settings values from to avoid
+ *     hitting the settings store
+ */
+export default function shouldHideEvent(ev: MatrixEvent, ctx?: IState): boolean {
+    // Accessing the settings store directly can be expensive if done frequently,
+    // so we should prefer using cached values if a RoomContext is available
+    const isEnabled = ctx ?
+        name => ctx[name] :
+        name => SettingsStore.getValue(name, ev.getRoomId());
 
     // Hide redacted events
     if (ev.isRedacted() && !isEnabled('showRedactions')) return true;

test/components/structures/MessagePanel-test.js

@@ -51,8 +51,20 @@ class WrappedMessagePanel extends React.Component {
     };
 
     render() {
+        const roomContext = {
+            room,
+            roomId: room.roomId,
+            canReact: true,
+            canReply: true,
+            showReadReceipts: true,
+            showRedactions: false,
+            showJoinLeaves: false,
+            showAvatarChanges: false,
+            showDisplaynameChanges: true,
+        };
+
         return <MatrixClientContext.Provider value={client}>
-            <RoomContext.Provider value={{ canReact: true, canReply: true, room, roomId: room.roomId }}>
+            <RoomContext.Provider value={roomContext}>
                 <MessagePanel room={room} {...this.props} resizeNotifier={this.state.resizeNotifier} />
             </RoomContext.Provider>
         </MatrixClientContext.Provider>;