From 04e910bd9a7070553d7d401d6d9352619a0147b6 Mon Sep 17 00:00:00 2001 From: Constantin Kaplinsky Date: Tue, 25 Dec 2007 18:10:35 +0000 Subject: [PATCH] Added a numner of FIXME comments, to identify some possible improvements. git-svn-id: svn://svn.code.sf.net/p/tigervnc/code/trunk@2380 3789f03b-4d11-0410-bbf8-ca57d06f2519 --- unix/x0vncserver/PollingManager.cxx | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/unix/x0vncserver/PollingManager.cxx b/unix/x0vncserver/PollingManager.cxx index bf13cac6..841ce0f7 100644 --- a/unix/x0vncserver/PollingManager.cxx +++ b/unix/x0vncserver/PollingManager.cxx @@ -39,6 +39,8 @@ const int PollingManager::m_pollingOrder[32] = { 19, 3, 27, 11, 29, 13, 5, 21 }; +// FIXME: Check that the parameter's value is in the allowed range. +// This applies to all other parameters as well. IntParameter PollingManager::m_videoPriority("VideoPriority", "Priority of sending updates for video area (0..8)", 2); @@ -154,6 +156,10 @@ bool PollingManager::pollScreen() // "video passes" will be performed between normal polling passes. // No actual polling is performed in a video pass since we know that // video is changing continuously. + // + // FIXME: Should we move this block into a separate function? + // FIXME: Giving higher priority to video area lengthens video + // detection cycles. Should we do something with that? if ((int)m_videoPriority > 1 && !m_videoRect.is_empty()) { if (m_numVideoPasses > 0) { m_numVideoPasses--; @@ -168,6 +174,8 @@ bool PollingManager::pollScreen() // changeFlags[] array will hold boolean values corresponding to // each 32x32 tile. If a value is true, then we've detected a change // in that tile. Initially, we fill in the array with zero values. + // + // FIXME: Should we use a member variable in place of changeFlags? bool *changeFlags = new bool[m_widthTiles * m_heightTiles]; memset(changeFlags, 0, m_widthTiles * m_heightTiles * sizeof(bool)); @@ -189,6 +197,7 @@ bool PollingManager::pollScreen() haveVideoRect = handleVideo(changeFlags); // Inform the server about the changes. + // // FIXME: It's possible that (nTilesChanged != 0) but changeFlags[] // array is empty. That's possible because handleVideo() // modifies changeFlags[]. @@ -278,6 +287,11 @@ bool PollingManager::handleVideo(bool *pChangeFlags) memset(m_rateMatrix, 0, numTiles); } + // FIXME: It looks like the code below rather belongs to + // pollScreen(). Perhaps handleVideo() should be merged back + // to pollScreen(), and then pollScreen() should be split in + // some better way, if needed at all. + // Grab the pixels of video area. Also, exclude video rectangle from // pChangeFlags[], to prevent grabbing the same pixels twice. if (!m_videoRect.is_empty()) { -- 2.39.5