From 9bafa1d7ad911025f53be9a09a5504bc3ef2ebae Mon Sep 17 00:00:00 2001 From: Lyubomir Marinov Date: Thu, 24 Mar 2011 12:42:19 +0000 Subject: Fixes a layout and painting issue (seen on Mac OS X) which may lead to toggled local video in calls appearing to be of the wrong bounds. --- .../impl/gui/main/call/OneToOneCallPeerPanel.java | 10 +- .../impl/neomedia/MediaConfiguration.java | 146 ++++++++++++++------- .../impl/neomedia/MediaServiceImpl.java | 16 ++- .../jmfext/media/renderer/video/JAWTRenderer.java | 12 +- .../communicator/util/swing/VideoContainer.java | 74 +++++++++-- 5 files changed, 193 insertions(+), 65 deletions(-) diff --git a/src/net/java/sip/communicator/impl/gui/main/call/OneToOneCallPeerPanel.java b/src/net/java/sip/communicator/impl/gui/main/call/OneToOneCallPeerPanel.java index 2eced0c..dcbe743 100644 --- a/src/net/java/sip/communicator/impl/gui/main/call/OneToOneCallPeerPanel.java +++ b/src/net/java/sip/communicator/impl/gui/main/call/OneToOneCallPeerPanel.java @@ -933,7 +933,7 @@ public class OneToOneCallPeerPanel * Handles the change when we turn on/off local video streaming such as * creating/releasing visual component. * - * @param listener Listener that will be callbacked + * @param listener Listener that will be called back */ private void handleLocalVideoStreamingChange( VideoTelephonyListener listener) @@ -1782,6 +1782,14 @@ public class OneToOneCallPeerPanel videoContainer.remove(localVideo); videoContainer.remove(closeButton); } + + /* + * Just like #handleVideoEvent(VideoEvent, Container) says, + * we have to be explicit in order to achieve a proper + * layout and an up-to-date painting. + */ + videoContainer.validate(); + videoContainer.repaint(); } } } diff --git a/src/net/java/sip/communicator/impl/neomedia/MediaConfiguration.java b/src/net/java/sip/communicator/impl/neomedia/MediaConfiguration.java index b80e171..0c77a7f 100644 --- a/src/net/java/sip/communicator/impl/neomedia/MediaConfiguration.java +++ b/src/net/java/sip/communicator/impl/neomedia/MediaConfiguration.java @@ -5,14 +5,12 @@ */ package net.java.sip.communicator.impl.neomedia; -import java.util.*; - import java.awt.*; import java.awt.event.*; import java.io.*; import javax.media.*; -import javax.media.MediaException; +import javax.media.MediaException; // disambiguation import javax.swing.*; import javax.swing.event.*; import javax.swing.table.*; @@ -25,8 +23,9 @@ import net.java.sip.communicator.util.*; import net.java.sip.communicator.util.swing.*; /** - * @author Lubomir Marinov + * @author Lyubomir Marinov * @author Damian Minkov + * @author Yana Stamcheva */ public class MediaConfiguration { @@ -134,9 +133,11 @@ public class MediaConfiguration final SIPCommCheckBox echoCancelCheckBox = new SIPCommCheckBox( NeomediaActivator.getResources().getI18NString( "impl.media.configform.ECHOCANCEL")); - // first set the selected one than add the listener - // in order to avoid saving tha value when using the default one - // and only showing to user without modification + /* + * First set the selected one, then add the listener in order to avoid + * saving the value when using the default one and only showing to user + * without modification. + */ echoCancelCheckBox.setSelected( mediaService.getDeviceConfiguration().isEchoCancel()); echoCancelCheckBox.addItemListener( @@ -155,9 +156,11 @@ public class MediaConfiguration final SIPCommCheckBox denoiseCheckBox = new SIPCommCheckBox( NeomediaActivator.getResources().getI18NString( "impl.media.configform.DENOISE")); - // first set the selected one than add the listener - // in order to avoid saving tha value when using the default one - // and only showing to user without modification + /* + * First set the selected one, then add the listener in order to avoid + * saving the value when using the default one and only showing to user + * without modification. + */ denoiseCheckBox.setSelected( mediaService.getDeviceConfiguration().isDenoise()); denoiseCheckBox.addItemListener( @@ -207,10 +210,10 @@ public class MediaConfiguration { public void itemStateChanged(ItemEvent e) { - if(e.getStateChange() == ItemEvent.SELECTED) + if (ItemEvent.SELECTED == e.getStateChange()) { - if(DeviceConfiguration - .AUDIO_SYSTEM_PORTAUDIO.equals(e.getItem())) + if (DeviceConfiguration.AUDIO_SYSTEM_PORTAUDIO.equals( + e.getItem())) { createPortAudioControls(portAudioPanel); } @@ -224,8 +227,8 @@ public class MediaConfiguration } } }); - if(comboBox.getSelectedItem() - .equals(DeviceConfiguration.AUDIO_SYSTEM_PORTAUDIO)) + if (DeviceConfiguration.AUDIO_SYSTEM_PORTAUDIO.equals( + comboBox.getSelectedItem())) createPortAudioControls(portAudioPanel); } else @@ -372,14 +375,16 @@ public class MediaConfiguration } /** - * Creates preview for the device(video) in the video container. + * Creates preview for the (video) device in the video container. + * * @param device the device - * @param videoContainer the container - * @throws IOException a problem accessing the device. - * @throws MediaException a problem getting preview. + * @param videoContainer the video container + * @throws IOException a problem accessing the device + * @throws MediaException a problem getting preview */ - private static void createPreview(CaptureDeviceInfo device, - final JComponent videoContainer) + private static void createPreview( + CaptureDeviceInfo device, + JComponent videoContainer) throws IOException, MediaException { @@ -391,22 +396,23 @@ public class MediaConfiguration if (device == null) return; - Iterator mDevsIter = - NeomediaActivator.getMediaServiceImpl() - .getDevices(MediaType.VIDEO, MediaUseCase.ANY) - .iterator(); - while(mDevsIter.hasNext()) + MediaService mediaService = NeomediaActivator.getMediaServiceImpl(); + Iterable devs + = mediaService.getDevices(MediaType.VIDEO, MediaUseCase.ANY); + + for (MediaDevice dev : devs) { - MediaDeviceImpl dev = (MediaDeviceImpl)mDevsIter.next(); - if(dev.getCaptureDeviceInfo().equals(device)) + if(((MediaDeviceImpl) dev).getCaptureDeviceInfo().equals(device)) { - Component c = (Component)NeomediaActivator.getMediaServiceImpl() - .getVideoPreviewComponent( - dev, - videoContainer.getSize().width, - videoContainer.getSize().height); + Dimension videoContainerSize = videoContainer.getSize(); + Component preview + = (Component) + mediaService.getVideoPreviewComponent( + dev, + videoContainerSize.width, + videoContainerSize.height); - videoContainer.add(c); + videoContainer.add(preview); break; } @@ -492,31 +498,81 @@ public class MediaConfiguration * described by pretending there's a selection in the video combo * box when the combo box in question becomes displayable. */ - comboBox.addHierarchyListener(new HierarchyListener() + HierarchyListener hierarchyListener = new HierarchyListener() { + private Window window; + + private WindowListener windowListener; + + public void dispose() + { + if (windowListener != null) + { + if (window != null) + { + window.removeWindowListener(windowListener); + window = null; + } + windowListener = null; + } + + videoDeviceInPreview = null; + } + public void hierarchyChanged(HierarchyEvent event) { - if (((event.getChangeFlags() - & HierarchyEvent.DISPLAYABILITY_CHANGED) - != 0) - && comboBox.isDisplayable()) + if ((event.getChangeFlags() + & HierarchyEvent.DISPLAYABILITY_CHANGED) + == 0) + return; + + if (comboBox.isDisplayable()) { - // let current changes end their execution - // and after that trigger action on combobox - SwingUtilities.invokeLater(new Runnable(){ + /* + * Let current changes end their execution and trigger + * action on combobox afterwards. + */ + SwingUtilities.invokeLater(new Runnable() + { public void run() { comboBoxListener.actionPerformed(null); } }); + + /* + * FIXME When the Options dialog closes on Mac OS X, the + * displayable property of the comboBox will not become + * false. Consequently, the next time the Options dialog + * opens, the displayable property will not change. + * Which will lead to no preview being created for the + * device selected in the comboBox. + */ + if (windowListener == null) + { + window + = SwingUtilities.windowForComponent(comboBox); + if (window != null) + { + windowListener = new WindowAdapter() + { + @Override + public void windowClosing(WindowEvent event) + { + dispose(); + } + }; + window.addWindowListener(windowListener); + } + } } else { - if(!comboBox.isDisplayable()) - videoDeviceInPreview = null; + dispose(); } } - }); + }; + comboBox.addHierarchyListener(hierarchyListener); } else preview = new TransparentPanel(); diff --git a/src/net/java/sip/communicator/impl/neomedia/MediaServiceImpl.java b/src/net/java/sip/communicator/impl/neomedia/MediaServiceImpl.java index 71bf547..f150646 100644 --- a/src/net/java/sip/communicator/impl/neomedia/MediaServiceImpl.java +++ b/src/net/java/sip/communicator/impl/neomedia/MediaServiceImpl.java @@ -692,8 +692,9 @@ public class MediaServiceImpl * @param preferredHeight the height we prefer for the component * @return the preview component. */ - public Object getVideoPreviewComponent(MediaDevice device - , int preferredWidth, int preferredHeight) + public Object getVideoPreviewComponent( + MediaDevice device, + int preferredWidth, int preferredHeight) { JLabel noPreview = new JLabel(NeomediaActivator.getResources().getI18NString( @@ -720,16 +721,17 @@ public class MediaServiceImpl ((MediaDeviceImpl)device).getCaptureDeviceInfo().getLocator()); /* - * Don't let the size be uselessly small just because the videoContainer - * has too small a preferred size. - */ + * Don't let the size be uselessly small just because the + * videoContainer has too small a preferred size. + */ if ((preferredWidth < 128) || (preferredHeight < 96)) { preferredHeight = 128; preferredWidth = 96; } - VideoMediaStreamImpl - .selectVideoSize(dataSource, preferredWidth, preferredHeight); + VideoMediaStreamImpl.selectVideoSize( + dataSource, + preferredWidth, preferredHeight); // A Player is documented to be created on a connected DataSource. dataSource.connect(); diff --git a/src/net/java/sip/communicator/impl/neomedia/jmfext/media/renderer/video/JAWTRenderer.java b/src/net/java/sip/communicator/impl/neomedia/jmfext/media/renderer/video/JAWTRenderer.java index c21b0cb..7647b1e 100644 --- a/src/net/java/sip/communicator/impl/neomedia/jmfext/media/renderer/video/JAWTRenderer.java +++ b/src/net/java/sip/communicator/impl/neomedia/jmfext/media/renderer/video/JAWTRenderer.java @@ -683,7 +683,15 @@ public class JAWTRenderer @Override public void componentResized(ComponentEvent e) { - processLightweightComponentEvent(); + /* + * It is necessary to call + * #procesLightweightComponentEvent() when the parent gets + * resized only if the native counterpart of this + * SwingVideoComponent expects bounds in a coordinate system + * which changes with respect to the bounds of this + * SwingVideoComponent when the parent gets resized. + */ + //processLightweightComponentEvent(); } }; @@ -927,7 +935,7 @@ public class JAWTRenderer { /** * The handle to the native JAWTRenderer which does the actual - * paiting of this SwingVideoComponentCanvas. + * painting of this SwingVideoComponentCanvas. */ private long handle; diff --git a/src/net/java/sip/communicator/util/swing/VideoContainer.java b/src/net/java/sip/communicator/util/swing/VideoContainer.java index a387a7a..2042337 100644 --- a/src/net/java/sip/communicator/util/swing/VideoContainer.java +++ b/src/net/java/sip/communicator/util/swing/VideoContainer.java @@ -9,8 +9,6 @@ package net.java.sip.communicator.util.swing; import java.awt.*; import java.lang.reflect.*; -import javax.swing.*; - import net.java.sip.communicator.util.*; /** @@ -32,6 +30,15 @@ public class VideoContainer private static final Logger logger = Logger.getLogger(VideoContainer.class); /** + * The name of the instance method of Components added to + * VideoContainer which creates a new Component acting as + * a canvas in which the other Components contained in the + * VideoContainer are painted. + */ + private static final String VIDEO_CANVAS_FACTORY_METHOD_NAME + = "createCanvas"; + + /** * The Component which is the canvas, if any, in which the other * Components contained in this VideoContainer are * painted. @@ -102,6 +109,14 @@ public class VideoContainer @Override public void add(Component comp, Object constraints, int index) { + if (VideoLayout.CENTER_REMOTE.equals(constraints) + && (noVideoComponent != null) + && !noVideoComponent.equals(comp)) + { + remove(noVideoComponent); + validate(); + } + if ((canvas == null) || (canvas.getParent() != this)) { if (OSUtils.IS_MAC && (comp != canvas)) @@ -119,7 +134,9 @@ public class VideoContainer try { - Method m = comp.getClass().getMethod("createCanvas"); + Method m + = comp.getClass().getMethod( + VIDEO_CANVAS_FACTORY_METHOD_NAME); if (m != null) { @@ -179,13 +196,11 @@ public class VideoContainer index++; } - if (VideoLayout.CENTER_REMOTE.equals(constraints) - && (noVideoComponent != null) - && !noVideoComponent.equals(comp)) - { - remove(noVideoComponent); - validate(); - } + /* + * XXX Do not call #remove(Component) beyond this point and before + * #add(Component, Object, int) because #removeCanvasIfNecessary() will + * remove the canvas. + */ super.add(comp, constraints, index); } @@ -204,7 +219,10 @@ public class VideoContainer if ((comp == canvas) && (canvas != null) && (canvas.getParent() != this)) + { canvas = null; + validate(); + } if (VideoLayout.CENTER_REMOTE.equals( ((VideoLayout) getLayout()).getComponentConstraints( @@ -215,6 +233,8 @@ public class VideoContainer add(noVideoComponent, VideoLayout.CENTER_REMOTE); validate(); } + + removeCanvasIfNecessary(); } /** @@ -237,4 +257,38 @@ public class VideoContainer add(noVideoComponent, VideoLayout.CENTER_REMOTE); validate(); } + + /** + * Removes {@link #canvas} from this VideoContainer if no sibling + * Component needs it. + */ + public void removeCanvasIfNecessary() + { + if ((canvas == null) || !OSUtils.IS_MAC) + return; + + boolean removeCanvas = true; + + for (Component component : getComponents()) + { + if (component == canvas) + continue; + try + { + component.getClass().getMethod( + VIDEO_CANVAS_FACTORY_METHOD_NAME); + removeCanvas = false; + break; + } + catch (NoSuchMethodException nsme) + { + /* + * Ignore it because we already presume that component does not + * need the canvas. + */ + } + } + if (removeCanvas) + remove(canvas); + } } -- cgit v1.1